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\$ Jul 15, 2020 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\$ Jul 16, 2020 at 7:11
  • \$\begingroup\$ @BrettDeWoody That right there should be a comment ;) \$\endgroup\$
    – konijn
    Jul 16, 2020 at 13:46

2 Answers 2

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\$ Jul 16, 2020 at 17:13
0
\$\begingroup\$

I have a similar approach to the answer from Konijn, but my "Sort Functionion Factory " is a little more verbose since I wanted to pass a list of sort keys and allow for descending sort order.

The particular problem I was solving was for a generic "table view" of a chunk of data from a Firebase realtime database query.

The data I want to display will arrive at my Vue app like this example JSON

    {"someKidsPartyId": {
        "entry1": {"age": 6, "name": "John"},
        "entry2": {"age": 4, "name": "Jade"},
        "entry3": {"name": "Alfie"},
        "entry4": {"age": 5, "name": "Charlotte"}
      }
    }

I would first pre-process this into a simpler array like:-

    [{key: "entry1", age:6, name:"John"}, {key: "entry2", ...}}

But then I want to be ableto apply aribtrary sort specifications. Additionally, I want my sort function cope with data of different types for the same property name and also null etc.

So, my builder function uses "bind" to partially apply a text list of property names and returns a function that can be used in an array sort method.

The function (buildObjectSortFunctionFromPropertyList) will be used like this:-

  let fnSort = buildObjectSortFunctionFromPropertyList('age desc, name')
  myObjectArray.sort(fnSort)

Note that the routine will trim any whitespace in the list so that sorting os performed upon the "age" and "name" properties of the objects and not the "age " or " name" properties which will not exist at all!

/**
 * @description 
 * @param {string} propertyList - property names separated by commas to apply to a list of objects for sorting e.g. "age desc, name"
 * @returns {function} of type (a,b) => n where n is -1, 0, 1
 */
export function buildObjectSortFunctionFromPropertyList(propertyList) { 
     
     let fnSort = function (sortProp, x, y) {
          let aSortParts = sortProp.split(',').map(p => p.trim()) // e.g. " age desc, name" => ["age desc", "name"]
          let reIsDesc = /\sdesc$/i // does text end " desc" or " DeSC" etc.
          let aTypeOrder = ['boolean', 'number', 'string', 'object', 'undefined'] // When the pairs don't match will use this precedence order
          try {
     
               for (let i = 0; i < aSortParts.length; i++) {
               let sortPart = aSortParts[i]
               let sortFactor = 1
               if (reIsDesc.test(sortPart)) {
                    sortPart =  sortPart.substr(0, sortPart.length - 5)
                    sortFactor = -1
               } else {
                    sortFactor = 1
               }
               let xVal = x[sortPart]
               let yVal = y[sortPart]
               let xType = typeof xVal  
               let yType = typeof yVal
     
               let compareResult
               if (xType === yType) {
                    switch(xType) {
                    case 'string':
                         compareResult = xVal.localeCompare(yVal)
                         break;
                    case 'number':
                         compareResult =  xVal === yVal ? 0 : xVal < yVal ? -1 : 1
                         break;
                    case 'boolean':
                         compareResult = xVal === yVal ? 0 : xVal === false ? -1 : 1
                         break;
                    case 'object':
                         if (xVal === null && yVal === null) {
                         compareResult = 0
                         } else if (xVal === null) {
                         compareResult = -1
                         } else if (yVal === null) {
                         compareResult = 1
                         } else {
                         xVal = xVal.toString()
                         yVal = yVal.toString()
                         compareResult = xVal.localeCompare(yVal)
                         }
                         break;
                    default:
                         // This is typical of undefined, which will happen a lot if the passed property name is mistyped etc.
                         // So, just assume that the items pairs cannot be distinguished and use 0
                         compareResult = 0
                    }
               } else {
                    if (xVal === null && yVal === null) {
                    compareResult = 0
                    } else if (xVal === null) {
                    compareResult = 1 // Sort NULL to last
                    } else if (yVal === null) {
                    compareResult = -1 // Sort NULL to last
                    } else {
                    let xValOrder = aTypeOrder.findIndex(xType)
                    let yValOrder = aTypeOrder.findIndex(yType)
                    compareResult = xValOrder === yValOrder ? 0 : xValOrder < yValOrder ? -1 : 1
                    }
                    if (compareResult !== 0) {
                    return compareResult // a definte sorting decision is available but IGNORE sort factor! This case is precendent
                    }
               }
               if (compareResult !== 0) {
                    return compareResult * sortFactor// a definte sorting decision is available
               }  
               }
               
          } catch(ex) {
          
          }
          return 0 // Treat the items as equivalent havingnot found a specific difference in the list of sorting properties
     
     }
     return fnSort.bind(this, propertyList)
}
```
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to the Code Review Community. Good answers contain observations about the code in the original question, for an example see the bullet point list that starts off KonJin's answer. If you want a review of your code please post it as a question rather than an answer. Alternate solutions are not considered good answers on Code Review. \$\endgroup\$
    – pacmaninbw
    Dec 30, 2021 at 0:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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