Hello, everyone! Hope everybody is having a great coding time.
So, I was watching a video on why you should not use 'else’s with your if statements in order to improve readability and leave your code cleaner and not so much nested that you don’t know what is happening.
With that in mind, I tried to improve the code of updateRecords() function. I feel it’s better than having an if inside an if, however and I’m bothered with one ‘return’ for each if.
How would you guys improve it? Any suggestions?
Thanks in advance!
// Setup
const recordCollection = {
2548: {
albumTitle: 'Slippery When Wet',
artist: 'Bon Jovi',
tracks: ['Let It Rock', 'You Give Love a Bad Name']
},
2468: {
albumTitle: '1999',
artist: 'Prince',
tracks: ['1999', 'Little Red Corvette']
},
1245: {
artist: 'Robert Palmer',
tracks: []
},
5439: {
albumTitle: 'ABBA Gold'
}
};
// Only change code below this line
function updateRecords(records, id, prop, value) {
//Returns in every "if" so it will stop when finding the case
if (value == "") {
delete records[id][prop];
return records;
}
if (prop == "tracks" && records[id].hasOwnProperty(prop)) {
records[id][prop].push(value);
return records;
}
if (prop == "tracks" && !records[id].hasOwnProperty(prop)) {
records[id][prop] = [value];
return records;
}
if (prop != "tracks") {
records[id][prop] = value;
return records;
}
}
updateRecords(recordCollection, 5439, 'artist', 'ABBA');
**Your browser information:**
User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.2 Safari/605.1.15
I would not consider this an absolute, written in stone programming commandment. In general I agree with the sentiment in the video, but you can always take some things to the extreme. The example he used in the video was definitely set up to be able to get rid of the else statements, but it won’t always be that easy. I think maybe a better approach is to “prefer” early returns over else statements when appropriate.
As to your changes, I’m not sure how much you gain by changing the structure from
if
else if
else if
else if
to
if
if
if
if
You’ve basically removed three words.
The main thing you have changed is that you now have four return statements inside of each if instead of just one at the bottom. The requirement of this function is that it always returns a records object, and I believe that yours does (assuming the object being passed in contains the id). But if I were looking at the code for the first time, or after a long time away from it, my first thought would be that this function returns either a records object or undefined and I would have to do some work to figure out that the if statements cover every possible scenario. So I think it could be argued that your all if version introduces some ambiguity as to what the function returns. Of course you could add a return records at the bottom to make it clearer, but then why have all those other returns in the if statements?
Definitely try to avoid long complicated if/else statements when you can, but don’t take it to the extreme where you possibly introduce other issues.
Dogmatically following any ‘coding rule’ is going to produce bad code. You should write code that is clear and makes sense… In this case, repeating the same line over and over is a red flag that something is more complicated than it should be in your code.