CODEWITHSUNDEEP
javascriptreactjsreduxreact-reduxselector
Shh
Shh

Reputation: 309

React JS: Refactoring Redux Selectors

I am using several selectors to get the boundaries on which to set my Google Maps. They basically return the lowest and highest lat/lng points from the data. The code works but I feel this is really messy and can be re-factored, but i'm not too sure how.

Here is the code:

const getMinLat = data =>
  data.reduce((prev, current) => {
    return prev.coords[0] < current.coords[0] ? prev : current
  })

const getMaxLat = data =>
  data.reduce((prev, current) => {
    return prev.coords[0] > current.coords[0] ? prev : current
  })

const getMinLng = data =>
  data.reduce((prev, current) => {
    return prev.coords[1] < current.coords[1] ? prev : current
  })

const getMaxLng = data =>
  data.reduce((prev, current) => {
    return prev.coords[1] > current.coords[1] ? prev : current
  })

const getBounds = data => [
  {
    lat: getMinLat(data).coords[0],
    lng: getMinLng(data).coords[1],
  },
  {
    lat: getMaxLat(data).coords[0],
    lng: getMaxLng(data).coords[1],
  },
]

Upvotes: 0

Views: 57

Answers (3)

Ori Drori
Ori Drori

Reputation: 191976

You can make it more concise by using Array.map():

const getCoordsArray = (data, index) =>
  data.map(o => o.coords[index]);

const getMin = (data, index) =>
  Math.min(...getCoordsArray(data, index));

const getMax = (data, index) =>
  Math.max(...getCoordsArray(data, index));

const getBounds = data => [getMin, getMax]
  .map(m => ({
    lat: m(data, 0),
    lng: m(data, 1),
  }));

Upvotes: 1

skyboyer
skyboyer

Reputation: 23705

just traverse list once:

const getBounds = data => 
    data.reduce(
        ([
             {lat: minLat, lng: minLng}, 
             {lat: maxLat, lng: maxLng}
         ], 
         {coords: [lat, lng]}
        ) =>
       [
            {
                lat: Math.min(minLat, lat), 
                lng: Math.min(minLng, lng)
            }, 
            {
                lat: Math.max(maxLat, lat), 
                lng: Math.max(maxLng, lng)
            }
      ], 
      [{lat: Infinity, lng: Infinity}, {lat: -Infinity, lng: -Infinity}]
    )

I agree in advance that readability is not the advantage here. But it goes through list just once.

Also after getting used to destructuring syntax it's quite clear that Math.max and maxLng/maxLat goes together to decrease possibility of using wrong variable

[UPD] and there is no need to use Infinity/-Infinity as start values(I used them to highlight idea behind). for longitude/latitude we may use 180/-180 as most extreme values

Upvotes: 1

Safi Nettah
Safi Nettah

Reputation: 1170

Maybe this

const getMin = (data, index) =>
  data.reduce((prev, current) => (prev.coords[index] < current.coords[index] ? prev : current))
    .coords[index];

const getMax = (data, index) =>
  data.reduce((prev, current) => (prev.coords[index] > current.coords[index] ? prev : current))
    .coords[index];

const getBounds = data => [
  {
    lat: getMin(data, 0),
    lng: getMin(data, 1)
  },
  {
    lat: getMax(data, 0),
    lng: getMax(data, 1)
  }
];

Upvotes: 1

Related Questions