Problem with the robot-name exercise

Programming JavaScript
1

I have passed all 8 of the non-optional parts of the test suite.
I could only get it to throw an error when user tried to change robot name directly by providing a setter for the private property #name, such that if it was called, would throw an error. I checked this by running the exercise on node on my home system–if I don’t provide a setter, it won’t change name, but it continues to run any further code. Then I found that I couldn’t find community solutions that explicitly dealt with this. So I commented out my explicit forcing of an error and ran that through the test suite – it passed all 8 tests…what am I missing here?

2

Could you point out where it asks that you make it throw an error on name setting? I’m not seeing that in the instructions or tests.

3

test(‘internal name cannot be modified’, () => {
const modifyInternal = () => {
robot.name += ‘a modification’;
};
expect(modifyInternal).toThrow();
});

This is line 72 in the specs file.

4

You might want to look at Public class fields and setter in the MDM docs.

5

oops on the edited out text–I’ve now found the section on private instance fields, but my code for this test, although it does not change the field value, does not produce a syntax error–and yet it passes the test. This is what I don’t understand.

6

Her code fully passes when the class includes:

set name(iname) {  // Outside User Cannot Change BName
	throw new Error("You cannot control value of name")
}

The mystery is that the test ‘internal name cannot be modified’ still passes if this setter is commented out. Loading her class into an interactive REPL and manually running let robot = new Robot(); robot.name += 'hey'; console.log(robot.name); does not throw (which is as expected); and the name doesn’t change (also as expected).

Why does expect(modifyInternal).toThrow(); pass in that circumstance?

7

I think I see it now. That expect body is not actually executing the modifyInternal function.

Should be

expect(modifyInternal()).toThrow()
// ..................^^

Any javascript folks care to confirm or deny?

8

I tried to modify my home version of the specs file in the way you show
Now my code with the setter throwing an error does not work – it fails the test and I don’t understand its complaint, which I copy below:

  ● Robot › internal name cannot be modified

    You cannot control value of name

      15 |     }
      16 |     set name(iname) {                      // Outside User Cannot Change BName
    > 17 | 	throw new Error("You cannot control value of name")
         | 	      ^
      18 |     }
      19 |
      20 |     reset() {                              // Provide Robot w/ New Name

      at Robot.set name [as name] (robot-name.js:17:8)
      at modifyInternal (robot-name.spec.js:74:17)
      at Object.modifyInternal (robot-name.spec.js:76:14)
9

expect(fn).toThrow() is meant to call the provided function, so I’m not sure Glenn is on the right track. But it would be worth trying this construction:

expect(() => modifyInternal()).toThrow();

The error you’re getting right now implies that calling it this way somehow upsets the catch chain such that expect() fails to intercept the error. I don’t see how it would work better by encapsulating the function-under-test inside a trivial wrapper function, but that’s what the Expect doc says, so let’s try it.

10

Oh, yes I do see why it would work better.

expect(modifyInternal()).toThrow();

calls modifyInternal() in the context of this code, before even entering expect(); whereas:

expect(() => modifyInternal()).toThrow();

passes that inline function definition to expect() , which will call it from inside a try ... catch construct.

OTOH, I don’t see why that is especially better than the original:

expect(modifyInternal).toThrow();

– which is also passing a function which expect() will call, inside its protective try ... catch.

11

Yes. When I make this modification in the spec file, my code again passes all tests.

12

This should be fixed in exercises/practice/robot-name/robot-name.spec.js – the repo says it’s in some sort of state of not accepting user input, so I’m not going to try raising bug & PR.

13

Make a PR and then link to the PR here. A maintainer can reopen it.

14

Unsolicited PRs are generally not accepted. Issues which are discussed on the forum and you get a go ahead to create a PR - those are very welcome.

15

I just got a ‘go-ahead’, but how do I know whose go-ahead is official?

16

If someone was giving a go ahead when they wouldn’t, a moderator would take care of things.

17
1 Like