Skip to content

Scooter Hire code review #2

Description

@mandyWW

Well done Tosin, you have thought hard about your design and used concepts such as static to evidence your understanding of class versus instance variables. You have put a lot of effort into the JSDoc too and your tests are very well documented and concise, great work.

A few comments below:

  • Scooter - I know we 'hacked' this a bit yesterday however looking at the constructor again, I would remove the code which pushes the Scooter id onto the Hiring Station and instead, create an addScooter method which gets called AFTER the Scooter is fully constructed (by another class or test). You don't have to fix now but in future that pattern is preferred as we can guarantee the object is fully constructed
  • .gitignore should be in the Assignment folder as currently it is not stopping node_modules being committed

Questions (please answer on Slack)

  • you have a master and a main branch - is this intentional? I looked at the code on master - is that correct?
  • Scooter - repair method, does this work? Not sure I understand some of the logic like ChargingStation.length - there is no length on the class?
  • Scooter test - Send scooter damage - does this work? I would expect expect(ChargingStation.allScooter).toContain(aeroHead)
  • did you generate the JSDoc? If so you should have an out/ dir
  • can you add a screenshot of your coverage report pls

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions