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

Branches - Erika #49

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

Branches - Erika #49

wants to merge 35 commits into from

Conversation

emaust
Copy link

@emaust emaust commented Sep 12, 2019

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What was a design challenge that you encountered on this project? I spent a lot of time trying to decide what responsibilities the reservation class would be responsible for
What was a design decision you made that changed over time over the project? N/A - didn't get far enough to change any decisions I put in place
What was a concept you gained clarity on, or a learning that you'd like to share? I learned more about what it means to have each class have its own responsibility
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case? An example of a nominal test I wrote was for the duration method. It's nominal because I was testing for the most likely scenario
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case? I didn't write any tests for edge cases yet, but when I wrote duration I was considering the case of a person renting and checking out a room in the same day and how I would account for that
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I was diligent about writing individual tests before writing code for that portion of the overall program, but after the initial planning stages, I didn't rely on psuedocode as much as I should have and I think that was contributing to where I'm currently stalled.

@jmaddox19
Copy link

Hotel

What We're Looking For

Test Inspection

Workflow yes (X) n/ yes but no test / no
Wave 1
List rooms X
Reserve a room for a given date range X
Reserve a room (edge case) No
List reservations for a given date No
Calculate reservation price X
Invalid date range produces an error No
Test coverage Pretty good for the source code that is there :)
Wave 2 No
View available rooms for a given date range
Reserving a room that is not available produces an error
Test coverage
Wave 3 No
Create a block of rooms
Check if a block has rooms
Reserve a room from a block
Test coverage

Code Review

Baseline Feedback
Used git regularly Good job with committing regularly!
Answer comprehension questions Yes
Design
Each class is responsible for a single piece of the program Relatively unclear with how little code is there.
Classes are loosely coupled Relatively unclear with how little code is there.
Fundamentals
Names variables, classes and modules appropriately Yes
Understanding of variable scope - local vs instance No
Can create complex logical structures utilizing variables Relatively unclear with how little code is there.
Appropriately uses methods to break down tasks into smaller simpler tasks Relatively unclear with how little code is there.
Understands the differences between class and instance methods N/A
Appropriately uses iterators and Enumerable methods N/A
Appropriately writes and utilizes classes Sort of
Appropriately utilizes modules as a namespace No but not a big deal :)
Wrap Up
There is a refactors.txt file No
The file provides a roadmap to future changes No

Overall Feedback

Very far from complete but we've already talked about that. There are some indications of lack of understanding, but in areas we've also talked about since submitting this assignment. Definitely worth taking the time to redo this like we've talked about :)

lib/service.rb Outdated


def room_cost(duration)
@cost = (duration - 1) * price

Choose a reason for hiding this comment

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

This is not useful as an instance variable here because it's specific to a specific room, not to the service instance.

lib/time.rb Outdated
@duration = duration
end

def reserve_time(date1, date2)

Choose a reason for hiding this comment

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

The code in this method seems to indicate a lack of understanding around instance variables.
Every line except the last one isn't updating the values.

lib/time.rb Outdated
attr_reader :check_in, :check_out, :duration

def initialize
@check_in = check_in

Choose a reason for hiding this comment

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

This doesn't make sense here because you aren't taking params in the initialize method.

lib/time.rb Outdated
require_relative '../lib/reservation'
require_relative '../lib/service'

class Dates

Choose a reason for hiding this comment

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

This whole class is pretty wonky. If it's not clear to you now what makes it wonky, I definitely wanna walk through that with you

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