I’m really happy with my code for this project, it works great, but I’m unsure if a couple lines should be changed.
function updateRecords(id, prop, value) {
if (value === "") {
delete collection[id][prop];
} else if (prop === "tracks") {
if (!collection[id].hasOwnProperty("tracks")) {
collection[id].tracks = []; // maybe this should say collection[id][prop] = [];
}
collection[id].tracks.push(value); // likewise this could say collection[id][prop].push(value);
} else {
collection[id][prop] = value;
}
return collection;
}
My question isn’t about dot vs bracket notation, but if I should continue to keep referencing the data using a implicitly with a variable, vs explicitly referenced as tracks data. I can make a good case for and against each way, both work fine, but each could introduce bugs in different ways if I make a mistake, get lost. (my actual code is commented, I’ve removed those comments because they were distracting from the actual question/concern.)
Your browser information:
User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36.
Link to the challenge: doesn’t allow me to link because I’m new to the forums, this is the best I can do:
freecodecamp.[org]/javascript-algorithms-and-data-structures/basic-javascript/record-collection
There’s no really right or wrong in this situations, it’s more a matter of personal experience.
The way I see it’s a good balance between naming (which is probably the hardest thing about programming) and reusability / maintainability.
In this case it’s pretty trivial but imagine a scenario where this function is uses in a big company with many developers:
1 - If your function would have been called
updateTracksInRecordCollection
then someone would expect it will only update tracks, likely no one will “accidentally” try to use it to update something else
2 - The requirement changes, and now the company added a new property in the collection, but it behaves exactly as tracks.
In this case your code require (little) more tweaking since it assumes tracks will always be the props.
However if you’d use a variable all you had to do would simply add the new prop case to the if statement
At the end of the day if your codes works it’s what’s matters the most
Yeah, I finished the lesson yesterday, but I’ve been thinking about that part of it all day today, and both ways bug me a little. One way is more readable, one way is more reuseable. I guess when the code is well commented (like it was originally), then I don’t need to prioritize readability. Thanks for the advise, and for the welcome.
You are much better off starting with this line: let record = collection[id]
Then instead of constantly referencing collection[id] throughout your code, reference record instead. Nearly every single solution I’ve seen to this challenge has people avoiding using intermediate variables like this, so I wonder if we might need some kind of refactoring lessons before this. The cause might be also due to the confusion of not knowing that record is a reference to the nested object, so a lesson reinforcing that would also be warranted.
And it might be irrelevant with the new curriculum, might not… ¯\(ツ)/¯
Yeah, that is way easier, had no idea that would be a possible solution. It improves readability and is easier to write, and I could do something similar for collection[id][prop]. It’s easy to get lost down the rabbit hole trying to keep my place as I get further in the nest.
New curriculum? Where can I find out more about that? because it would be kind of tedious to have all my progress tossed out the window and have to start over.
