3
\$\begingroup\$

I'm doing this assignment were they have told me to sort an array of objects with name and age. What do you think about my solution?

Edited to take in consideration comments, Thanks for the feedback guys!

var familyAgesPropName = [
    { name: "Raul", age: 27 },
    { name: "Jose", age: 55 },
    { name: "Maria", age: 52 },
    { name: "Jesus", age: 18 },
    { name: "Neo", age: 2 }
];

var familyAgesWithoutPropName = [
    { "Raul": 27 },
    { "Jose": 55 },
    { "Maria": 52 },
    { "Jesus": 18 },
    { "Neo": 2 }
];

var familyAgesWithoutPropNameMissingAge = [
    { "Raul": 27 },
    { "Jose": 55 },
    { "Maria": '' },
    { "Jesus": 18 },
    { "Neo": 2 }
];

var familyAgesWithoutPropNameMissingName = [
    { "Raul": 27 },
    { "Jose": 55 },
    { 52: "" },
    { "Jesus": 18 },
    { "Neo": 2 }
];

var familyAgesWithoutPropNameMissingNameAndNULL = [
    null,
    { "Raul": 27 },
    { "Jose": 55 },
    { 52: "" },
    { "Jesus": 18 },
    { "Neo": 2 }
];

/**
    @brief: cleaningAndFormatting is a function that takes the input array (that I assume can come in any way) and converts it
    to a proper format that is correct for using and outputing it. The format of my choice is [{name: String, age: Int}, item2, ...]
    @param: array with the data.
    @notes: If the input array comes already in the desired format we can comment this function improving the performance of the process
            If the name is actually a number (only digits) then we put it infront to see that we have a problem with it
            If the age is empty or a string that doesn't make sense we assign 0 to put it after the problmatics
**/

// var cleaningAndFormatting = (function(array) {
//  for (var i = array.length - 1; i >= 0; i--) {
//      if (array[i].name === undefined) {
//          var tempObject = {};
//          for (var key in array[i]) {
//              tempObject.name = key;
//              tempObject.age = parseInt(array[i][key]) || 0;
//              if (!isNaN(tempObject.name)) {
//                  tempObject.age = -1;
//              }
//              if (isNaN(tempObject.age)) {
//                  tempObject.age = 0;
//              }
//          }
//          array[i] = tempObject;
//      }
//  }
// });

function cleanRow(element, index, array) {

    if (element == null) {
        delete array[index];
        return;
    }
    if (element.name == undefined) {
        element.name = Object.keys(element)[0];
    }
    if (element.age == undefined) {
        element.age = element[element.name];
        element.age = parseInt(element.age) || 0;
    }
    if (!isNaN(element.name)) {
        element.age = -1;
    }
    delete element[element.name];
}

familyAgesPropName.forEach(cleanRow);
console.log("familyAgesPropName");
console.log(familyAgesPropName);
familyAgesWithoutPropName.forEach(cleanRow);
console.log("familyAgesWithoutPropName");
console.log(familyAgesWithoutPropName);
familyAgesWithoutPropNameMissingAge.forEach(cleanRow);
console.log("familyAgesWithoutPropNameMissingAge");
console.log(familyAgesWithoutPropNameMissingAge);
familyAgesWithoutPropNameMissingName.forEach(cleanRow);
console.log("familyAgesWithoutPropNameMissingName");
console.log(familyAgesWithoutPropNameMissingName);
familyAgesWithoutPropNameMissingNameAndNULL.forEach(cleanRow);
console.log("familyAgesWithoutPropNameMissingNameAndNULL");
console.log(familyAgesWithoutPropNameMissingNameAndNULL);


/**
    @brief: Manual implementation of the quicksort algorithm adapted our desired array, I've chosen do the algorithm manually because the sorting in JavaScript is
    very dependant on the implementation of the engine that runs the JavaScript making it erratic and not desirable to use. For example chrome V8 engine for JavaScript
    unstable.

**/

var quickSort = (function() {

    function partition(array, left, right) {
        var cmp = array[right - 1].age,
            minEnd = left,
            maxEnd;
        for (maxEnd = left; maxEnd < right - 1; maxEnd += 1) {
            if (array[maxEnd].age <= cmp) {
                swap(array, maxEnd, minEnd);
                minEnd += 1;
            }
        }
        swap(array, minEnd, right - 1);
        return minEnd;
    }

    function swap(array, i, j) {
        var temp = array[i];
        array[i] = array[j];
        array[j] = temp;
        return array;
    }

    function quickSort(array, left, right) {
        if (left < right) {
            var p = partition(array, left, right);
            quickSort(array, left, p);
            quickSort(array, p + 1, right);
        }
        return array;
    }

    return function(array) {
        return quickSort(array, 0, array.length);
    };
}());

quickSort(familyAgesPropName);
quickSort(familyAgesWithoutPropName);
quickSort(familyAgesWithoutPropNameMissingAge);
quickSort(familyAgesWithoutPropNameMissingName);
\$\endgroup\$
5
  • \$\begingroup\$ I'm wondering why you have familyAgesPropName and familyAgesWithoutPropName as well. As they entirely contain similar information except from "age" . This is putting some stress on cleaningAndFormatting function \$\endgroup\$
    – Tolani
    Aug 18, 2016 at 17:29
  • \$\begingroup\$ Well, the idea behind the familyAges are that the cleanAndFormatting can handle different input errors and output good format no mather what \$\endgroup\$
    – Trouner
    Aug 18, 2016 at 17:31
  • \$\begingroup\$ To say in essence that sort() implementations are erratic, and that you will write your own improved sort routine sounds very uninformed. I would advise you not to write this type of comments in interview or school assignments. \$\endgroup\$
    – konijn
    Aug 19, 2016 at 12:02
  • \$\begingroup\$ Technically it's accurate since sort is implemented different in different engines, that was my point. I've made quite research before writing that and found that for example chrome engine V8 has an unstable sort. Ref: stackoverflow.com/questions/3026281/… \$\endgroup\$
    – Trouner
    Aug 19, 2016 at 13:39
  • \$\begingroup\$ @Trouner point taken. But then I would mention the unstable sort in the comment. \$\endgroup\$
    – konijn
    Aug 19, 2016 at 17:00

2 Answers 2

2
\$\begingroup\$

Based on your implementation, I have no other choice than to say welcome to JavaScript . I will point out few observations

cleaningAndFormatting():

  • null is a valid value for an object in javascript. For instance replacing one of the object with a null e.g
var familyAgesPropName3 = [
    null,
    {name: "Jose", age: 55},
    {name: "Maria", age: 52},
    {name: "Jesus", age: 18},
    {name: "Neo", age: 2}
];

will generate this error

Can not read Property-Uncaught TypeError

  • Type–Converting Comparison (==): converts the operands to the same type before making the comparison. So undefined== null will return true as opposed to false. In Javascript, we use the strict comparison (e.g., ===) to return only true if the operands are of the same type and the contents match. You can readmore from this page Comparison Operators

  • Javascript provides ForEach function which can replace the nested for..loop . I will give you a pseudocode on how to start

 function houseKeeping(element, index, array) {

   if (element.name == undefined) {

     element.name = element.age;
     //  other implementations
   }
   //  other implementations
 }

To use the ForEach

/* Calling the foreach*/
familyAgesPropName.forEach(houseKeeping);
  • for( var key in array[i]){..} is inefficient as the loop is performed twice; key in the first iteration is name and second key. You should see your error now . tempObject.name = key; is assigned itself and after the key-Horrendous Implementation
  • I'm not sure of what you are trying to achieve with this line
    if (!isNaN(tempObject.name)) {...}.

An excerpt from isNan() explains how NaN values are generated:
NaN values are generated when arithmetic operations result in undefined or unrepresentable values. Such values do not necessarily represent overflow conditions. A NaN also results from attempted coercion to numeric values of non-numeric values for which no primitive numeric value is available.

There was no arithmetic operation conducted on tempObject.name except tempObject.age, I doubt if that line is necessary. It would be nice if you could explain what you mean by this and I will update my answer if necessary

If the name is actually a number (only digits) then we put it infront to see that we have a problem with it

  • console.log(array);: I believe you want to return the modified array rather than printing to the console . You can replace that with return array;
  • tempObject: you don't need to create an extra variable when you could just modify the array itself

quickSort()

I just looked at your quicksort briefly. Although I will be back to give further reviews , I will leave you with a note here for now

  • Destructing Assignment : I'm not sure how conversant you are with this type of assignment in javascript. This will save you some coding lines meaning you don't have to have a swap function what you could do is replace
swap(array, minEnd, right - 1);

// replace with
array[minEnd, right - 1] = array [right - 1,minEnd]

I will back later in the day. I hope this helps.

\$\endgroup\$
1
  • \$\begingroup\$ Tried to use the Destructing Assignment and didn't work very good. I'll try later this weekend again to see if I can make it work. All other comments were really helpfull, thanks! \$\endgroup\$
    – Trouner
    Aug 19, 2016 at 18:26
1
\$\begingroup\$

Either your assignment was something else or you overcomplicate things needlessly.

JavaScript built-in array sort function to the rescue:

familyAgesPropName.sort(function compareProps(a, b) {
    return (a && a.age |0) - (b && b.age |0);
});

[familyAgesWithoutPropName,
 familyAgesWithoutPropNameMissingAge,
 familyAgesWithoutPropNameMissingName,
].forEach(function(array) {
    array.sort(function compareNoProps(a, b) {
        return (a[Object.keys(a)[0]] |0) - (b[Object.keys(b)[0]] |0);
    });
});

In the absence of explicit instructions for invalid data, I use a 0 fallback and treat age as integer. Other possibilities: filter the arrays with .filter(), throw an exception, return a list of errors.

\$\endgroup\$
3
  • \$\begingroup\$ Eh, going with keys(a)[0] seems over-complicated as well, still +1 for advising built-in sort() \$\endgroup\$
    – konijn
    Aug 19, 2016 at 12:04
  • \$\begingroup\$ Well, Object.keys is concise comparing to for-in loop and seems quite self-evident to me in comparison. Of course, I would think twice before using it in production code, but if it would turn out to be as fast as for-in, I'd probably use it. \$\endgroup\$
    – wOxxOm
    Aug 19, 2016 at 16:44
  • \$\begingroup\$ I did overcomplicated it, because I wanted to show that I took the assignment seriously. \$\endgroup\$
    – Trouner
    Aug 19, 2016 at 18:24

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.