Tell us what’s happening:
My code is passing the test, but I feel that it is quite messy - it could be written more concisely and in simpler way. What should I do/learn to achieve it?
Your code so far
// Setup
var collection = {
2548: {
album: "Slippery When Wet",
artist: "Bon Jovi",
tracks: [
"Let It Rock",
"You Give Love a Bad Name"
]
},
2468: {
album: "1999",
artist: "Prince",
tracks: [
"1999",
"Little Red Corvette"
]
},
1245: {
artist: "Robert Palmer",
tracks: [ ]
},
5439: {
album: "ABBA Gold"
}
};
// Only change code below this line
function updateRecords(id, prop, value) {
if (prop != "tracks" && value != ""){
collection[id][prop] = value;
}
if (prop == "tracks" && !('tracks' in collection[id]))
{
// console.log("No tracks property here!");
collection[id][prop] = [];
}
if (prop == "tracks" && value != ""){
// console.log("There is tracks property here");
collection[id][prop].push(value);
}
if ([value] == ""){
delete collection[id][prop];
}
return collection;
}
updateRecords(5439, "artist", "ABBA");
Your browser information:
User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.149 Safari/537.36.
One thing that you could do is some nesting. For example, you have three different checks for whether prop is “tracks”. You could write a single if/else for that check, and then use nested ifs as needed.
All I did, is I moved checking if value is empty to the top and I deleted checking if value is not empty from if (prop == "tracks") instead of this after checking if there is 'tracks' property, it’s pushing new value into 'tracks', also modified second else if.
Does it help in anyway? I feel like now my code is more complicated and harder to understand.
My code:
function updateRecords(id, prop, value)
{
//I nested ifs here to write more concise code
if (value == "")
{
delete collection[id][prop];
}
else if (prop == "tracks")
{
if (!('tracks' in collection[id]))
{
collection[id][prop] = [];
}
collection[id][prop].push(value);
}
else if (prop != "tracks")
{
collection[id][prop] = value;
}
return collection;
}
I find your code difficult to read only because of how you use new lines: don’t put the opening graph parenthesis of the if statement, or even of the function, on a new line. If you ever find yourself with something that authomatically put semicolons at the end of lines you will find yourself with this:
if (...);
{ ... }
function func(...);
{ ... }
and it will throw errors.
Instead, if we nicely format everything, it is much easier to read:
function updateRecords(id, prop, value) {
//I nested ifs here to write more concise code
if (value == "") {
delete collection[id][prop];
} else if (prop == "tracks") {
if (!("tracks" in collection[id])) {
collection[id][prop] = [];
}
collection[id][prop].push(value);
} else if (prop != "tracks") {
collection[id][prop] = value;
}
return collection;
}
the only other thing you can do is change the last else if to just an else
if (value === "") {
// delete
}
else {
if (prop === "tracks") {
if (!("tracks" in collection[id])) { // I'd actually use hasOwnProperty, but whatevere
// make empty array
}
// push
}
else {
// assign value
}
}
Why would I organize my logic like this? Largely to reduce repetition. I only have to check each rule once. That means that if the rule changes, it’s easier to update my code. It reduces the likelihood of me making a stupid error. It makes each individual if statement easier to read. It also means that the computer doesn’t have to make the same check repeatedly, so it’s (imperceptibly) more efficient.
When you’re new to this, nested statements might seem move confusing to read, but as you get more familiar with them you actually get pretty good at scanning them like decision trees.
[is the value ""?]
/ \
yes no
/ \
remove [is prop "tracks"?]
/ \
yes no
/ \
[does "tracks" exist?] assign
/ \
yes no
/ |
create it |
\ /
push
Wow, your code and explanation actually so clean and easy to understand even for beginner like me
It make sense from the point of modifying and updating code.
You even showed your logic behind this and explained decision tree.
I’m super grateful for the time and effort you gave to explain it to me.
