2
\$\begingroup\$

I'm building a functional React component that allows a list of products to be sorted by various properties. The properties could be top-level or nested properties. And I'd also like a sort order to be defined. As a secondary sort, I'd like a name property to be used.

I have left out the React parts to boil it down to the plain JS.

Here's my first take on it:

sort = [one-of-the-cases]
ascending = [true||false]

const sortFunc = (a, b) => {
  let aParam = null
  let bParam = null
  switch (sort) {
    case 'brand':
      aParam = a.brand.title
      bParam = b.brand.title
      break
    case 'weight':
      aParam = a.metadata.weight
      bParam = b.metadata.weight
      break
    case 'price':
      aParam = a.price.unit_amount
      bParam = b.price.unit_amount
      break
    case 'style':
      aParam = a.metadata.style
      bParam = b.metadata.style
      break
    case 'arrival':
      aParam = b.created
      bParam = a.created
      break
    default:
      aParam = a.brand.title
      bParam = b.brand.title
      break
  }

  // Sort by property, ascending or descending
  if (aParam < bParam) {
    return ascending ? -1 : 1
  }
  if (aParam > bParam) {
    return ascending ? 1 : -1
  }
  
  // Sort by name
  if (a.name < b.name) {
    return -1
  }
  if (a.name > b.name) {
    return 1
  }

  return 0
}

products.sort(sortFunc)

This seems overly complicated. One idea I have is to flatten the object to eliminate the nested properties, which would make it possible to store the sort param as a variable, and use bracket notation to reference the object property. This would eliminate the switch statement.

Any other ideas for simplifying this complicated sort?

\$\endgroup\$
3
  • \$\begingroup\$ Did you omit the ascending intentionally from the name comparison? \$\endgroup\$ – Roland Illig Jul 15 '20 at 22:16
  • \$\begingroup\$ For now, yes. The name comparison is to order by name in the case there are matches in the first comparison. \$\endgroup\$ – Brett DeWoody Jul 16 '20 at 7:11
  • \$\begingroup\$ @BrettDeWoody That right there should be a comment ;) \$\endgroup\$ – konijn Jul 16 '20 at 13:46
1
\$\begingroup\$

From a short review;

  • ascending should probably be a parameter in the sortFunc
  • sort (sortKey) should probably be a parameter in the sortFunc
  • I feel things should be Spartan (1 char) or spelled out
    • sortFunc -> sortFunction -> sorter?
  • I would strongly consider the closure concept (example ion proposal)
  • I would harmonize the sort values with the object fields,or create a map
  • There is no need to initialize aParam and bParam with null, the default undefined should do
  • You probably want to add a comment as to why you compare name at the end
  • I prefer the if/else if approach even when if performs a return, it's one line less and increases readability

This is my counter-proposal;

const sortAscending = true;
const sortDescending = false;

function createSortFunction(key, ascending){

  const keyMap = {
    brand : 'brand.title',
    weight: 'metadata.weight',
    price: 'price.unit_amount',
    style: 'metadata.style',
    arrival: 'created'
  }
  
  key = keyMap[key]? keyMap[key] : 'brand.title';
  
  return (function sortFunction(a, b){
  
    //Minor magic, derive the property from a the dot notation string
    key.split('.').forEach(part=>{a = a[part], b = b[part];}); 
    
    // Sort by property, ascending or descending
    if (a < b) {
      return ascending ? -1 : 1
    }else if (b > a) {
      return ascending ? 1 : -1
    }
  
    // Sort by name if the properties are same
    if (a.name < b.name) {
      return -1
    } else if (a.name > b.name) {
      return 1
    }
    //a and b are equal
    return 0;
  });
}


products = [
 { name : 'Bob',   metadata: { weight: 160, style: 'sage'}, price: { unit_amount: 145}, created : '19750421', brand : { title: 'Sir' } },
 { name : 'Skeet', metadata: { weight: 130, style: 'ninja'}, price: { unit_amount: 160}, created : '20010611', brand : { title: 'Bro' } }
]

products.sort(createSortFunction('weight', sortAscending));
console.log(products);

\$\endgroup\$
1
  • \$\begingroup\$ I like the keyMap, that is super helpful, and the magic for deriving the property. But that causes the a.name reference to break because a and b have been reassigned to the property value. This can fixed easily though. \$\endgroup\$ – Brett DeWoody Jul 16 '20 at 17:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.