ChoreDoors Code Review

#1

I would appreciate it if someone could point out what is wrong with my isClicked() function and/or the if block in door1.onclick() ?

door1.onclick() doesn’t change the door1.src attribute after adding the if block.

const doorImage1 = document.getElementById('door1');
const doorImage2 = document.getElementById('door2');
const doorImage3 = document.getElementById('door3');

const imgPath = './img/';
const botDoorImg = imgPath + 'robot.svg';
const beachDoorImg = imgPath + 'beach.svg';
const spaceDoorImg = imgPath + 'space.svg';
const closedDoorImg = imgPath + 'closed_door.svg';

let numClosedDoors = 3;

let openDoor1;
let openDoor2;
let openDoor3;

function isClicked(door){
  if (door.src === closedDoorImg){
    return false;
  } else {
    return true;
  }
}

const playDoor = () => {
  numClosedDoors--;
  console.log(numClosedDoors);

  if (numClosedDoors === 0){
    //gameOver();
    alert('Game Over');
  }
}

const randomChoreDoorGenerator = () => {
  let choreDoor = Math.floor(Math.random() * numClosedDoors);

  if (choreDoor === 0) {
    openDoor1 = botDoorImg;
    openDoor2 = beachDoorImg;
    openDoor3 = spaceDoorImg;

  } else if (choreDoor === 1) {
    openDoor2 = botDoorImg;
    openDoor1 = beachDoorImg;
    openDoor3 = spaceDoorImg;

  } else if (choreDoor === 2){
    openDoor3 = botDoorImg;
    openDoor2 = beachDoorImg;
    openDoor1 = spaceDoorImg;
  }

}

door1.onclick = () => {
  if (!isClicked(door1)){
      door1.src = openDoor1;
      playDoor();
  }
}

door2.onclick = () => {
  door2.src = openDoor2;
  playDoor();
}

door3.onclick = () => {
  door3.src = openDoor3;
  playDoor();
}

randomChoreDoorGenerator();
1 Like
#2

What is door1? Did you call them from the dom by using getElementById? You might want to look into that ;).

1 Like
#3

Thank you for your reply.

Unless I’m not understanding something, yes I did, kindly refer to the first line of the code:
const doorImage1 = document.getElementById('door1');

1 Like
#4

True, but after that you are not calling doorImage1 anywhere in your code ;). Instead you are using the ids from the HTML and not the doorImage1, 2 and 3.

1 Like
#5

Btw your image location is wrong. The images are not stored in the same folder where the code is. Instead it is stored outside of the codecademy website.

const botDoorPath = "https://s3.amazonaws.com/codecademy-content/projects/chore-door/images/robot.svg";
const beachDoorPath = "https://s3.amazonaws.com/codecademy-content/projects/chore-door/images/beach.svg";
const spaceDoorPath = "https://s3.amazonaws.com/codecademy-content/projects/chore-door/images/space.svg";
const closedDoorPath = "https://s3.amazonaws.com/codecademy-content/projects/chore-door/images/closed_door.svg"
1 Like
#6

This doesn’t make any sense :frowning: The other doors without the if blocks are working.

door2.onclick = () => {
  door2.src = openDoor2;
  playDoor();
}

door3.onclick = () => {
  door3.src = openDoor3;
  playDoor();
}

Also, the paths are different from the instructions. I’ve saved the images locally and the code is running locally instead of on Code Academy . There’s nothing wrong with the image paths in the code.

1 Like
#7

Is the console giving any errors? (I can’t answer in the next 2 hours due to work meeting).

1 Like
#8

Not at all unfortunately. See below screenshot.
The output you see in the console is console.log(numClosedDoors); in the playDoor() function.

image
image.png937×680 32.9 KB

1 Like
#9

Right, so for some reason my code does work in the Code Academy editor, but not locally.

Edit: Modifying the image paths to the Code Academy S3 bucket solves my issue locally, so the problem lies with the relative paths somewhere. I’m curious to find out what it is exactly. If I ever do, I’ll post it here :slight_smile:

1 Like
#10

Thoses two are still referring to the globals, and not doorImageN.

If we can assume you’ve got the img folder in the same directory as index.html.

imgPath = "img/"

will work fine. It is rather moot, though.

botDoorImg = "img/robot.svg";
1 Like
#11

Thanks for the feedback.

Isn’t doorImageN a global variable as well? It’s declared in the global scope, no?

Even after taking your advice on the paths, the doors just don’t open when using local paths.
Could you please elaborate as to why this would be “moot” ?

1 Like
#12

Yes it is declared in global scope, in this example, but it is less likely some other code will collide with them because they are not in the HTML as id. Given we declare them as constants, they cannot collide with anythng.

Do you have the images stored in the img folder?

The variable you are assigning img/ to is longer than the actual path name. Simpler just to use it and not take up memory with a superfluous variable.

1 Like
#13

Thank you for your valuable input Sir, I now understand what you were pointing out regarding my first point on global scope.

As pointed out in my previous posts, the doors open fine, the images change when I don’t use the if block in the .onclick functions. As soon as the if block is there, it doesn’t work anymore.
I understand this is probably out of scope for Code Academy (since I’m trying this locally), but it does intrigue me why it’s not working.

As for your last point, I actually followed your advice in another post, basically saying DRY:

let someImage = "path/to/image.jpg";
let someImage2 = "path/to/image2.jpg";

should better be:

let imagePath = "path/to/";
let someImage = imagePath + "image.jpg";
let someImage2 = imagePath + "image2.jpg";
2 Likes
#14

That did look familiar… It would apply more to the long URLs in the track version. If the URL is a local path, then I would go with whatever is shortest, and not use a variable (which uses memory) if I didn’t have to. It’s not a deal breaker and it does help readability, to a certain degree.

#15

Just finished Chore Door project. It was really challenging but feel great completing it. Thanks guys! Nice project to tackle.

1 Like