Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Octos- Ari- Hotel #48

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Octos- Ari- Hotel #48

wants to merge 8 commits into from

Conversation

arrriiii
Copy link

@arrriiii arrriiii commented Mar 19, 2018

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision? I made a room class but wasn't sure if it had enough responsibility to be it's own class or a hash. I struggled with this idea--
Describe a concept that you gained more clarity on as you worked on this assignment. The idea of coupling dependencies became more clear as I wrote the reservation & room class. Moving forward, I would focus more on writing clear (working) code first and then manage dependencies.
Describe a nominal test that you wrote for this assignment. Creating a list of 20 rooms would be a nominal test as it's testing that a room can be created and is an instance of the Room class.
Describe an edge case test that you wrote for this assignment. I did not write an edge cases.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I could have wrote more pseudocode before writing actual code. I initially developed an outline of the different classes on paper and that was helpful.

@kariabancroft
Copy link

@ari-1 You need to answer the comprehension questions - then we can review the code!

end

def self.all
return @reservations
Copy link

@kariabancroft kariabancroft Mar 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot use instance variables in class methods because they refer to the overall class.

new_ticket[:start_date] = Date.parse(start_date)
new_ticket[:end_date] = Date.parse(end_date)

@total_rooms.find do |room|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use find here? Could you use room.sample by itself without the find method?

@kariabancroft
Copy link

Hotel

What We're Looking For

Feature Feedback
Design
Demonstrated classes having a single responsibility You structured your classes such that building out the functionality should keep them separated out.
Demonstrated loose coupling Yes
Methods demonstrate a good use of encapsulation, inputs and outputs Yes
Wave 1 requirements Yes
Wave 2 requirements No
Wave 3 requirements No

For a project of this length, you should be committing using git much more often. For the amount of code you had written, you had a very insufficient number of tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants