Record Collection Waypoint. Code discussion

1

My code works, but it feels clunky. Does anyone have any tips on how I could make it better?

If is for updating objects (albums) in a record collection. The data is incomplete. Albums are organized by id number, and within the id object they can have properties for album, artist and tracks - with tracks being an array where each item is a track title.
The function has the inputs: id, prop and value.
These correspond to album id number, the album property (alabum title, artist or tracks) and the value of the stated property.

Instructions:

  1. If prop isn’t “tracks” and value isn’t empty (""), update or set the value for that record album’s property.
  2. If prop is “tracks” but the album doesn’t have a “tracks” property, create an empty array before adding the new value to the album’s corresponding property.
  3. If prop is “tracks” and value isn’t empty (""), push the value onto the end of the album’s existing tracks array.
  4. If value is empty (""), delete the given prop property from the album.

Addressing no. 2 is where I feel my code is clunkiest.

MY CODE:

function updateRecords(id, prop, value) {

if (value === “”) {
delete collection[id][prop]; //no value to add, so delete the update attempt

} else if (prop !== “tracks” && value !== “”) {
collection[id][prop] = value;

//updates the value for a given record id property that isn’t the ‘tracks’ array -ie 5439,“album”,“new title”- changes the album value of id 5439 to “new title”

} else if (prop === “tracks”) {
var check = collection[id].hasOwnProperty(“tracks”); //does this album have a tracks property?
if (check === false) {
collection[id].tracks = []; //create tracks property for id
collection[id][prop].push(value); //add the value/first track to it

} else if (check === true) {
  collection[id][prop].push(value);    //adds new tracks to end of tracks list for given id
}

}
return collection;
}

1 Like
2

Let me post a formatted version of your code for easy reading.

function updateRecords(id, prop, value) {
  if (value === '') {
    delete collection[id][prop]; //no value to add, so delete the update attempt
  } else if (prop !== 'tracks' && value !== '') {
    collection[id][prop] = value;
    //updates the value for a given record id property that isn't the 'tracks' array
    //ie5439, 'album', 'new title' - changes the album value of id 5439 to 'new title'
  } else if (prop === 'tracks') {
    var check = collection[id].hasOwnProperty('tracks'); //does this album have a tracks property?
    if (check === false) {
      collection[id].tracks = []; //create tracks property for id
      collection[id][prop].push(value); //add the value/first track to it
    } else if (check === true) {
      collection[id][prop].push(value); //adds new tracks to end of tracks list for given id
    }
  }
  return collection;
}
2 Likes
3

thank you :slight_smile:

1 Like
4

You can rewrite the else if (prop === 'tracks') block as

var check = collection[id].hasOwnProperty('tracks');
if (!check) {
  collection[id].tracks = [];
}
collection[id][prop].push(value);

If the value to test in the if-condition is boolean, there’s no need to compare with false or true. You can put the value itself (or !value if testing for the negative).

Since both branches of the if-else block runs this line: collection[id].hasOwnProperty('tracks');, you can move it after the entire if-else block. This empties the else if block, so it’s safe to remove it.

2 Likes
5

cool thanks! Is this because the condition for the IF loop you stated (!check) only runs when it evaluates to true? So the boolean is implied in the expression in the condition statement?

1 Like
6

The body after the if will run if the condition is a “truthy” value.
The following values are “falsy”:

  • false
  • 0
  • '' (empty string)
  • null
  • undefined, and
  • NaN

Everything else is “truthy”.

2 Likes
7

This is probably just a matter of preference, but I left off creating a new variable and simply tested the value in the “if” like this:

else if (prop === ‘tracks’ && !collection[id].hasOwnProperty(‘tracks’)) {

They both work. I just felt this was simpler/cleaner to read.

Cheers!

8

I began this with what I thought was some seriously clunky code as well, basically trying to follow the if-then logic as laid out in the text explaining the challenge. As I began to clean it up by refactoring, I ultimately boiled it down to the following flowchart:

I ended up with the following code:

function updateRecords(id, prop, value) {
  if (value === "") {
    delete collection[id][prop];
  } else {
    if (prop === "tracks") {
      if (!collection[id].hasOwnProperty(prop)) {
        collection[id][prop] = [];
      }
      collection[id][prop].push(value);
    } else {
      collection[id][prop] = value;
    }
  }
  return collection;
}

which of course is very close to what you ended up with above.

2 Likes