Having problem with record collection

JavaScript
1

This is the code ive written,
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.

If prop is "tracks" and value isn’t empty ( "" ), push the value onto the end of the album’s existing tracks array.

these two conditions are not working for me

// 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"
    }
};
// Keep a copy of the collection for tests
var collectionCopy = JSON.parse(JSON.stringify(collection));

// Only change code below this line
function updateRecords(id, prop, value) {
  
   if(prop != "tracks" && value != ""){
    collection[id][prop] = value;
  }
  else if(prop == "tracks" && collection[id].hasOwnProperty("tracks") == "tracks"){
    collection[id][prop] = [];
    collection[id][prop] = value;
  }
  else if(prop == "tracks" && value != ""){
    collection[id][prop] = collection[id][prop].push(value);
  }
  else if(value == ""){
    delete collection[id][prop];
  }
  return collection;
}

// Alter values below to test your code
console.log(updateRecords(2468, "tracks", "Free"));
2

hasOwnProperty returns a boolean, which you are comparing to a string.

3

Ive updated the code

// 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"
    }
};
// Keep a copy of the collection for tests
var collectionCopy = JSON.parse(JSON.stringify(collection));

// Only change code below this line
function updateRecords(id, prop, value) {
  
   if(prop != "tracks" && value != ""){
    collection[id][prop][value] = value;
  }
  else if(prop == "tracks" && collection[id].hasOwnProperty("tracks") == true){
    collection[id][prop] = [];
    collection[id][prop] = value;
  }
  else if(prop == "tracks" && value != ""){
    collection[id][prop] = collection[id][prop].push(value);
  }
  else if(value == ""){
    delete collection[id][prop];
  }
  return collection;
}

// Alter values below to test your code
console.log(updateRecords(2468, "tracks", "Free"));
4

Look carefully. What are you setting to value here?

  else if(prop == "tracks" && collection[id].hasOwnProperty("tracks") == true){
    collection[id][prop] = [];
    collection[id][prop] = value;
  }
  else if(prop == "tracks" && value != ""){

The second else if will only run if (prop == "tracks" && collection[id].hasOwnProperty("tracks") == true) was false. The two tracks logic blocks will never both execute.

5

Can you explain in detail I am confused

6

EDIT: Took me a while to do this, other people had already answered so forgive any repetition.

Hey @topcoder

Firstly, you should probably use === and !== in JS (unless you actually want type coercion). Quickly just grabbed a post about this: Should I use === or == equality comparison operator in JavaScript?

hasOwnProperty returns true or false. Is there a need to then compare it to a string?

Maybe just use collection[id].hasOwnProperty(prop) because you’ve already established that prop == "tracks" just before the &&.

…sets the property to an empty array, then promptly replaces that with a string. You want the tracks property to be an array with that value in it? You could use [value].

…what does push return? Go check it out (under Syntax section): Array.prototype.push() - JavaScript | MDN

It returns something, which is then assigned to collection[id][prop]… which isn’t what you want. Maybe you don’t need to assign at all.

Without a spoiler, I structured it differently because I personally find else if statements hard to read, it just shows a different approach:

  // value is empty
  if () {
    /**
     * Delete property and bail out early. Empty value
     * is now taken out the equation
     */
    return collection;
  }

  // If prop is 'tracks'
  if () {
      // ...and the tracks property exists
    if () {
        // ...push the track onto the array
    }
    else {
        // ...create property and assign array containing the value
    }
  }
  else {
      // prop is not 'tracks', create or update prop with value
  }
  
  return collection;

I find this approach much simpler to digest. Hope this helps :slight_smile:

1 Like
7

This is the code Ive written ,its not working ,I think its clean

// 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"
    }
};
// Keep a copy of the collection for tests
var collectionCopy = JSON.parse(JSON.stringify(collection));

// Only change code below this line
function updateRecords(id, prop, value) {
    if(value === ""){
      delete collection[id][prop][value];
      return collection;
  }
  if(prop === "tracks"){
    collection[id].hasOwnProperty(prop) === true;
    if(collection[id].hasOwnProperty(prop)){
      collection[id][prop] = [];
      collection[id][prop] = [value];
    }
    else{
      collection[id][prop] = [value].push();
    }
  }
  else{
    collection[id][prop][value] = [value];
  }
  
  return collection;
}

// Alter values below to test your code
updateRecords(5439, "artist", "ABBA");
9

You can’t begin with
if (value === "")
You need to check for the prop and value first.
if there is no property with prop name, create it.
if there is a property with prop but the value is NOT empty, update it.
if there is a property with prop but the value IS empty, delete the prop.
Read the rules carefully, put them on paper first on write a pseudo-code.

1 Like
10

Seems to work ok for me. If the property exists and value is empty, it just deletes the property. Tested with:

updateRecords(5439, "artist", "ABBA"); // adds property/value
updateRecords(5439, "artist", ""); // no change
updateRecords(5439, "test", ""); // no change
updateRecords(5439, "test", "value"); // adds property/value

@topcoder
Maybe you could also make use of falsy values here for checking if value is empty: Falsy Values in JavaScript

This line:

delete collection[id][prop][value];

You want to delete the property itself, not the value of the property. That line is like:

delete collection['1245']['artist']['Robert Palmer']

No need for two lines here:

collection[id][prop] = [];
collection[id][prop] = [value];

Not sure this line needs to be in there, what’s it doing?

collection[id].hasOwnProperty(prop) === true;

This statement appears to be the wrong way round:

// the value has the property, so push onto the array
if(collection[id].hasOwnProperty(prop)){
    collection[id][prop] = [];
    collection[id][prop] = [value];
}
else{
    collection[id][prop] = [value].push();
}

Lastly, in the final else:

collection[id][prop][value] = [value];

You want to assign that [value] to the property.

Well done for keeping at it. As @padunk says, it could be really helpful to plan it out beforehand on paper.

11

No, it won’t work, see his code:

if(value === ""){
      delete collection[id][prop][value];
      return collection;
  }

He didn’t check for prop, so your test case will fail

updateRecords(5439, "artist", ""); // it should delete property artist 
updateRecords(5439, "test", ""); // it should delete property test

The rule is when value is empty then you have to delete the property.
This mean that you need to check if the prop exist and value IS empty then you can delete it.

12

@padunk Yeah maybe I could have been clearer, I was referring to my own code block. I can see that:

if (value === "") {
    delete collection[id][prop][value];
    return collection;
}

…wouldn’t work. But it’s the code inside the block that’s the problem. I didn’t have to check if the property exists first. See: delete - JavaScript | MDN

Namely:

The delete operator removes a given property from an object. On successful deletion, it will return true , else false will be returned. However, it is important to consider the following scenarios:

  • If the property which you are trying to delete does not exist, delete will not have any effect and will return true
// using vscode quokka plugin here
const car = {
    brand: 'BMW',
    colour: 'silver',
    wheels: 4
}

delete car['topSpeed'];
car // { brand: 'BMW', colour: 'silver', wheels: 4 }

delete car['colour'];
car // { brand: 'BMW', wheels: 4 }

I think in this case, it makes perfect sense to check for an empty value and deal with that first, rather than chaining a bunch of if - else if - else. If the value is empty, what’s the point in even going further into the function? This of course is just my personal preference.

13

I see what you mean, couldn’t agree more. :+1:

1 Like
15

I know, but @topcoder needs to work this out. That’s not my code. I was just pointing out that the if/else was the wrong way round. The next step would have been working out why the push doesn’t work. Probably need to wait and see what the current state of the code is, may have completed this task by now.

16

Sorry, I wanted to answer to the OP, I didn’t want to direct that to you - probably weird thing since I quoted your post

17

You can’t use push in that way

18

No worries. Maybe instead of quoting, just use @topcoder then manually put the code into markdown that you want their attention drawn to. I think this thread is getting tricky and is possibly confusing the OP more than helping! We probably need to see the latest full code and go from there :slight_smile: