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);
2 Answers 2
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 anobject
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. Soundefined== null
will returntrue
as opposed tofalse
. In Javascript, we use thestrict comparison (e.g., ===)
to return onlytrue
if the operands are of the same type and the contents match. You can readmore from this page Comparison OperatorsJavascript provides
ForEach
function which can replace the nestedfor..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 isname
and secondkey
. You should see your error now .tempObject.name = key;
is assigned itself and after thekey
-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 inundefined
or unrepresentable values. Such values do not necessarily represent overflow conditions. ANaN
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 withreturn 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 aswap
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.
-
\$\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– Trouner2016年08月19日 18:26:35 +00:00Commented Aug 19, 2016 at 18:26
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.
-
\$\begingroup\$ Eh, going with
keys(a)[0]
seems over-complicated as well, still +1 for advising built-insort()
\$\endgroup\$konijn– konijn2016年08月19日 12:04:38 +00:00Commented 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– woxxom2016年08月19日 16:44:29 +00:00Commented Aug 19, 2016 at 16:44
-
\$\begingroup\$ I did overcomplicated it, because I wanted to show that I took the assignment seriously. \$\endgroup\$Trouner– Trouner2016年08月19日 18:24:57 +00:00Commented Aug 19, 2016 at 18:24
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\$