Time - Sharon Cheung

[sharonkeikei]

Assignment Submission: Hotel

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Question	Answer
What was a design challenge that you encountered on this project?	I think the most challenge part is to figure out where to begin with. I think the Design Scaffolding really helped me a lot!!! I was very lost in the beginning and mixed up all the concepts that we learned in this past month, I have mistaken the Object inheritance as a composition, just all ideas mixed up. But I learnt from this project that I have to be patient with myself and don't panic before I even start!
What was a design decision you made that changed over time over the project?	I took it really slow in the beginning to make sure that's the direction that I would like to take first. So there are not much major changes in the design.
What was a concept you gained clarity on, or a learning that you'd like to share?	I learned a lot from this project! The most important lesson is to think!!! I always have this problem of waiting for instruction. Not having the direct step by step instruction forced me to really think about how I would like the program to do first.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case?	I have some nominal cases in my test for each class to make sure it is creating the instance that I want. A nominal test case is a type of test case that describes a piece of core functionality needed for the success of this method. This is the test case that verifies that the method does its primary responsibility.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case?	I have some edge case to test every time an Argument Error should raise, for example, when there is no such a room in the hotel block to reserve. An edge test case is a type of test case that verifies that the method can work successfully, even given non-obvious context. This is the test case that verifies that the method works, even with very unexpected context.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	To be honest, I have never really written pseudocode before this project, I tended to just dive in and try to write the codes, which makes me frustrated sometimes when I didn't get it to work. But for this project, I really took my time to sit down and think of what I want and how to write the code and wrote it down in plain English for each method before I get started, and it amused me how useful pseudocode are! For writing the test first, I am still working on that, I tend to write the code first and cannot think backward to start with the test.

[beccaelenzil]

Hotel

Section 1: Major Learning Goals

Criteria	yes/no, and optionally any details/lines of code to reference
Practices SRP by having at least two separate classes with distinct responsibilities, and test files for these two classes	✔️
Overall, demonstrates understanding instance variables vs. local variables. (There aren't unnecessarily too many instance variables, when it should be a local variable)	✔️
For each test file, tests demonstrate an understanding of instantiating objects properly, and using Arrange-Act-Assert	✔️
Practices pseudocode and TDD, and reflected on it by filling out the reflection questions	✔️
Practices git with at least 15 small commits and meaningful commit messages	✔️

Section 2: Code Review and Testing Requirements

Criteria	yes/no, and optionally any details/lines of code to reference
There is a class that represents a reservation, and a second class that holds/manages a collection of reservations through composition (instance variable)	✔️
The logic for checking if a reservation's date overlaps with another reservation's date is complex logic that is separated into method(s) (and potentially class(es))	✔️
The logic for checking if a reservation's date overlaps with another reservation's date has unit tests	✔️See in-line comment
All of the given tests run and pass	✔️
A test coverage tool is installed and used, and shows 95% test coverage	✔️

Section 3: Feature Requirements

Feature Requirement: There is a method that...	yes/no
gives back a list of rooms, and it's tested	✔️
creates a specific reservation for a room for a given date range, and it has nominal test cases	✔️
creates a specific reservation for a room for a given date range, and it tests an edge case, such as no available room, or invalid date range	More edge case tests are needed.
gives back a list of reservations on a given date. Its tests include a test that makes several reservations for a given date	✔️
calculates the total price for a reservation	I see a test for a single reservation. You should check for multiple reservations.
gives back a list of available rooms for a given date range, and it has nominal test cases	✔️
gives back a list of available rooms for a given date range, and it has edge test cases, such as no available room, or invalid date range	Edge case tests are needed
creates a block of rooms	✔️
reserves a room from a block	✔️See in-line comment for suggestion of how to fully test this.

Overall Feedback

Overall Feedback	Criteria	yes/no
Green (Meets/Exceeds Standards)	14+ total in all sections

Additional Feedback

Great work overall! You've built your first project with minimal starting code. This represents an incredible milestone in your journey, and you should be proud of yourself!

I am particularly impressed by the way that you designed your classes and methods to have single responsibility. You've done a great job working through some tricky logic.

I do see some room for improvement around the thoroughness of your tests. It is important to test multiple nominal and edge success and failure cases to ensure that your code correctly handles different scenarios.

Keep up the hard work!

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation	✅
Elegant/Clever	✅
Descriptive/Readable	✅
Concise	✅
Logical/Organized	✅

[beccaelenzil]

Consider putting this exception raising functionality into the DateRange class.

[beccaelenzil]

Minor note: consider finding the available room before creating a new reservation and using this available room when instantiating the new reservation.

[beccaelenzil]

Consider the fact that you don't necessarily need to check if occupied rooms are empty. @room_list - occupied_rooms_list will return the correct array whether or not occupied_rooms_list is empty. Reducing the number of tests makes your code faster and more concise.

[beccaelenzil]

These tests do a good job testing the nominal cases. It is important to provide numerous test cases for each method, testing possible edge cases. For instance, what do reserve_room and 'available rooms` return when there are no available rooms?

[beccaelenzil]

It looks like you are testing an edge case here. Nice work! You should put this in it's own it block so it is clear what you are testing.

[beccaelenzil]

It is hard for me to see from you tests whether you've accounted for the situation where a reservation is trying to be made for a room that is reserved in a block. This, and other cases where regular reservation and block reservations may overlap should be explicitly tested for.

[beccaelenzil]

Minor syntax note: tab in the content of an it block.

[beccaelenzil]

These are good nominal case tests. It is important to thoroughly test the different ways that dates can overlap. See this pseudo code from the design-scaffold

  describe "overlap?" do
    before do
      start_date = Date.new(2017, 01, 01)
      end_date = start_date + 3

      @range = Hotel::DateRange.new(start_date, end_date)
    end

    it "returns true for the same range" do
      start_date = @range.start_date
      end_date = @range.end_date
      test_range = Hotel::DateRange.new(start_date, end_date)

      expect(@range.overlap?(test_range)).must_equal true
    end

    xit "returns true for a contained range" do
    end

    xit "returns true for a range that overlaps in front" do
    end

    xit "returns true for a range that overlaps in the back" do
    end

    xit "returns true for a containing range" do
    end

    xit "returns false for a range starting on the end_date date" do
    end

    xit "returns false for a range ending on the start_date date" do
    end

    xit "returns false for a range completely before" do
    end

    xit "returns false for a date completely after" do
    end
  end

  xdescribe "include?" do
    it "reutrns false if the date is clearly out" do
    end

    it "returns true for dates in the range" do
    end

    it "returns false for the end_date date" do
    end
  end