-2

I can't figure out why the following code always goes to my else statement, instead of my initial if condition.

var player = {
  inventory: [
    "Bullet"
  ],
};

function use() {
  alert(this); // Returns "Bullet"
  alert(player.inventory); // Returns "Bullet"
  if (this == player.inventory) {
    document.getElementById("inventory02").innerHTML = "You USED" + this;
    alert("in inventory"); // She skips this

  } else {
    document.getElementById("inventory02").innerHTML = "You don't have" + this;
    alert("not in inventory"); // She always does this
  }
}
<button id="use_bullet" onclick="use.call('Bullet')">Use Bullet</button>
Krisztián Balla
  • 15,886
  • 11
  • 58
  • 69
  • @MaorRefaeli - I'm concerned that the restructuring of my original question "code" does not emulate the sequence/order of operations on my end, and to this point the snippet does not represent what happens when I run my own code using it's separate and external files. However, if you're confident separate and external JS files have no bearing then so be it. – JavaScripture Sep 02 '18 at 17:59
  • Splitting js code is a common practice (you have numerous bundle strategies and libraries that can help you bring code together to one file). Regardless, as long as the files are loaded and variables are available at their scope it doesn't matter that they are on separate files. – Maor Refaeli Sep 02 '18 at 18:08
  • OK. Just saying I wasn't getting that error for example. Anyway, I seem to run into far more trouble with separate and external files than you allude to, but I am determined to keep things on separate files (more like modules, one for objects, one for functions, etc) none-the-less. Here's to hoping my issues are less and less related to external files. THANKS for your help on all. – JavaScripture Sep 02 '18 at 18:33

1 Answers1

1
  1. player.inentory is an array, you need to check if the item is in the array and not equal to it.
  2. The use of this here is confusing (you are concatenating it, comparing it.. it's just wrong), you can pass an argument instead or better yet, keep an attribute on the element that represents the item and use it.
  3. Array.includes is not supported in IE. If you need it to run on IE and you don't transpile your js code (with Babel for instance) you can use polyfill or this answer.
  4. I replaced the alert with console.log since it's more clear in the example (and less annoying).
  5. You don't need that extra , after the declaration of inventory.

var player = {
  inventory: [
    "Bullet"
  ]
};

function use(e) {
  var item = e.target.getAttribute("item");
  console.log(item); // Returns "Bullet"
  console.log(player.inventory); // Returns "Bullet"

  if (player.inventory.includes(item)) {
    document.getElementById("inventory02").innerHTML = "You USED " + item;
    alert("in inventory"); // She skips this

  } else {
    document.getElementById("inventory02").innerHTML = "You don't have " + item;
    alert("not in inventory"); // She always does this
  }
}
<button id="use_bullet" item="Bullet" onclick="use(event)">Use Bullet</button>
<div id="inventory02"></div>

Edit:
According to the comments you prefer not to use an HTML attribute. I would still recommend avoiding this in this particular situation. Here is my suggestion:

var player = {
  inventory: [
    "Bullet"
  ]
};

function use(item) {
  console.log(item); // Returns "Bullet"
  console.log(player.inventory); // Returns "Bullet"

  if (player.inventory.includes(item)) {
    document.getElementById("inventory02").innerHTML = "You USED " + item;
    alert("in inventory"); // She skips this

  } else {
    document.getElementById("inventory02").innerHTML = "You don't have " + item;
    alert("not in inventory"); // She always does this
  }
}
<button id="use_bullet" onclick="use('Bullet')">Use Bullet</button>
<div id="inventory02"></div>
Maor Refaeli
  • 2,024
  • 2
  • 18
  • 31
  • if "if (this === player.inventory[0])" this is the only line you changed, it didnt help. Same results on my end. Please note I am using external js files as indicated as it may affect your testing. EDIT: Looks like you changed your answer substantially from the one I was commenting on. Can't wait to try. – JavaScripture Sep 02 '18 at 08:19
  • Yeah, i saw why you wanted it as an array. you can also press on "Run code snippet" (the blue button) and try. – Maor Refaeli Sep 02 '18 at 08:22
  • My use of "this" is because I am going to eventually be "picking up" (pushing) many items (objects) into my inventory (object array)... the hardcoding of "Bullet" into my inventory array was just for debugging. Also the extra comma was left over because the player object will eventually have far more inside it than just the inventory array. (hence the shape of my original code) -- Given these things, is your approach still valid? – JavaScripture Sep 02 '18 at 17:23
  • **I'm trying to keep my js code as least reliant on HTML as possible (can't recall the term for that). – JavaScripture Sep 02 '18 at 17:32
  • If you want to rely on HTML as much as possible isn't an `item` attribute the best solution for you? However, if you prefer to keep using `use.call("Bullet")` or `use("Bullet")` i'll edit my answer – Maor Refaeli Sep 02 '18 at 17:36
  • Well I'm not sure as it seemed to add another tag HTML had to deal with, and I just wanted you to have the most info as possible on my situation. Also full disclosure @OmidNavy solution worked perfectly out of the gate for me with current code, HOWEVER, it fell part as soon as I added an additional (different) item into my inventory, so the jury is still out. -- And either way I wouldn't delete your existing code as a possible alternative (I still need to explore). Thanks for your efforts. – JavaScripture Sep 02 '18 at 17:46
  • I believe you should *teach a man to fish and you give him an occupation that will feed him for a lifetime*. The use of `call` here is redundant and overkill, also `this` should suggest on the context object not a parameter passed to a function. I prefer to keep it as simple as possible. Check out the edit. – Maor Refaeli Sep 02 '18 at 17:56
  • In your second version... is "item" defined somewhere? – JavaScripture Sep 02 '18 at 18:08
  • `item` is a parameter. When i wrote `use('Bullet')`, `Bullet` is passed to `item`. `this` in js is a tricky thing for newcomers and should be used appropriately. If you can, pass independent parameters to functions and keep them simple, this is the best thing you can do also for future refactoring. I think you should read some explanations about js. It's a weird and wonderful language. – Maor Refaeli Sep 02 '18 at 18:12
  • 1
    The advice to use independent parameters instead `this` to help future proof as I expand (and for easier debugging) seems invaluable. Your second suggestion keeps things more on the js side and works flawlessly, even after I introduced a second "item". I am in your debt... until next time! – JavaScripture Sep 02 '18 at 18:29
  • (Now I just need to figure out how to make that array item reference back to the item `object` I've created to access its properties!) *rolls up sleeves* :) – JavaScripture Sep 02 '18 at 18:38