-1

I created a project to check an array to update a car collection and came across an issue that I cannot explain. I'm relatively new to JS and still getting my bearings.

My code works fine, as in it gives me the results I want to see only if I call the function once. It is supposed to take a car that is not part of my carsOwned array, dump it into carsWanted and then display the carsWanted array.

If I call my function once, it will do all of this correctly. However if I call the function a second time with another vehicle that is not part of the carsOwned array. It's as if I never called my function the first time, it seems the array is being cleared after each time it is executed.

function updateCarCollection(cars) {
  let carsOwned = ["Lamborghini Aventador", "Ferrari 488", "Bentley Continental", "Audi RS7"]
  let carsWanted = []

  if (carsOwned.indexOf(cars) === -1) {
    carsWanted.push(cars);
    console.log("I don't have this car yet, but I added it to the cars I want.");
  } else if (carsOwned.indexOf(cars) > -1) {
    console.log("This car is already part of my collection");
  }

  console.log(carsWanted);
}

updateCarCollection("Ferrari 428");

//even though I called it the first time it will behave ad if I never called the function the first time  

updateCarCollection("Spyder 718");
t.niese
  • 39,256
  • 9
  • 74
  • 101
Vinny
  • 3
  • 1
  • 2
    Because you are recreating the array from scratch each time you call the function. `let carsWanted = []` is almost the first thing you do. – takendarkk Jun 27 '18 at 16:33
  • 1
    Another issue you should be wary of is that Javascript is pass-by-reference for everything except objects. So if you pass in `carsWanted` as an argument to the function, you still won't get the expected behavior. – Thomas Cohn Jun 27 '18 at 16:35
  • 1
    @Helium_1s2 I guess you wanted to say `for everything except primitives` and not `for everything except objects`. – t.niese Jun 27 '18 at 16:40
  • Yes, I was debating wording it as "pass by reference for everything except primitives" and "pass by value for everything except objects", and somehow used half of each one :) – Thomas Cohn Jun 27 '18 at 16:41
  • @Helium_1s2 and you should take care with the wording `pass by reference` [Does Javascript pass by reference?](https://stackoverflow.com/questions/13104494) (or - i know thats about Java, but still the same argumentation - [Is Java “pass-by-reference” or “pass-by-value”?](https://stackoverflow.com/questions/40480)) – t.niese Jun 27 '18 at 16:45

3 Answers3

0

You have to declare your variables outside of your function.

Otherwise each time you call your function carsWanted and carsOwned are reset to their initial value.

let carsOwned = ["Lamborghini Aventador", "Ferrari 488", "Bentley Continental", "Audi RS7"]
let carsWanted = []

function updateCarCollection(cars) {
  if (carsOwned.indexOf(cars) === -1) {
    carsWanted.push(cars);
    console.log("I don't have this car yet, but I added it to the cars I want.");
  } else if (carsOwned.indexOf(cars) > -1) {
    console.log("This car is already part of my collection");
  }

  console.log(carsWanted);
}

updateCarCollection("Ferrari 428");
// [ "Ferrari 428" ]

updateCarCollection("Spyder 718");
// [ "Ferrari 428", "Spyder 718" ]
anthowelc
  • 85
  • 1
  • 10
  • Thanks so much, in addition to your response I had someone explain it to me and now I can wrap my head around it. – Vinny Jun 27 '18 at 17:03
0

The issue you're having is due to the way you have declared your carsWanted array. By declaring it within your updateCarCollection function, you've scoped the variable to that function (see this short article on MDN about scoping). A simple way to think of it is like this: your carsWanted array is born, lives, and dies within the updateCarCollection. Every time your updateCarCollection method runs, it re-creates and re-initializes the carsWanted array to an empty array. It doesn't have any notion of what you've done previously to the carsWanted array, because the carsWanted array 'died' once your previous call to updateCarCollection was finished.

What you want to do here instead is scope your variable differently by moving it out of the updateCartCollection method. So you just need to update your code to something like this:

let carsWanted = []
function updateCarCollection(cars) {
    // ...
}

I'd also recommend turning carsOwned into a const IF (and only if) you never intend to modify that array.

  • Wow. Thanks so much for explaining it like this, it makes a lot more sense the way you stated it. I can actually wrap my head around this logic now. – Vinny Jun 27 '18 at 17:02
0

Work with an object you can reference. let creates a local only and goes away when the scope (in the function) it is declared in is gone(you "leave" the function.

So, let's create a "holder" object for our two arrays AND just use that to hold our function - that way it is nicely in our "namespace" of myCarThings Note that this inside the function refers to that myCarThings object now so inside that, we can reference it that way i.e. this.carsOwned. Now before adding it to the "wanted" list we probably also wish to check that we did not do that already. I added an example to show that also.

var myCarThings = {
  carsOwned: ["Lamborghini Aventador", "Ferrari 488", "Bentley Continental", "Audi RS7"],
  carsWanted: [],
  updateCarCollection: function(newCar) {
    if (this.carsOwned.indexOf(newCar) === -1) {
      if (this.carsWanted.indexOf(newCar) === -1) {
        this.carsWanted.push(newCar);
        console.log("I don't have this car yet, but I added it to the cars I want:", newCar);
      } else {
        console.log("I already said I want:", newCar);
      }
    } else if (this.carsOwned.indexOf(newCar) > -1) {
      console.log("This car is already part of my collection:", newCar);
    }
    console.log(this.carsWanted);
  }
};

myCarThings.updateCarCollection("Ferrari 428");

//even though I called it the first time it will behave ad if I never called the function the first time  

myCarThings.updateCarCollection("Spyder 718");
myCarThings.updateCarCollection("Bentley Continental");
// directly put one in
myCarThings.carsWanted.push("My New Jeeper");
// say I want it again
myCarThings.updateCarCollection("My New Jeeper");
Mark Schultheiss
  • 32,614
  • 12
  • 69
  • 100