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

Associations #4

Merged
merged 12 commits into from
Sep 4, 2023
Merged

Associations #4

merged 12 commits into from
Sep 4, 2023

Conversation

RileyManda
Copy link
Owner

@RileyManda RileyManda commented Sep 4, 2023

I have completed the following implementations on the School Library Application according to the stipulated requirements:

The following features were implemented:

Classes

  • Created a class Classroom with the following:@Label instance variable, initialized in the constructor.
  • Created a Setter and getter for @Label (using attr_accessor).
  • Created a class Rental with the following: @Date instance variable,initialized in the constructor.
  • Created a Setter and getter for @Date (using attr_accessor).
  • Created a class Book with the following:@title and @author instance variables
  • Set the classroom for the student.

Associations

  • Created the has-many side of Book and Rental (a book has many rentals).
  • Created the belongs-to side of Rental and Book (a rental belongs to a book).
  • Created the has-many side of Person and Rental (a person has many rentals).
  • Created the belongs-to side of Rental and Person (a rental belongs to a person).
  • Modified the constructor of Rental so Book and Person are set in it.

In addition, I have fulfilled the following project requirements:

General requirements

Tests

Please kindly review my implementation and suggest any changes and improvements.

I look forward to your feedback.

Thank you 🙏🏻

Best regards,

@RileyManda RileyManda changed the title Created class classroom with label instant var Associations Sep 4, 2023
Copy link

@Olamarx Olamarx left a comment

Choose a reason for hiding this comment

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

Hi @RileyManda,

Good job so far, indeed you have spent time on this project!
There are some issues that you still need to work on to go to the next project but you are almost there!

Highlights

  • Linters passed ✔️
  • Readme file professional ✔️
  • Commit messages meaningful ✔️

Required Changes ♻️

Check the comments under the review.

Optional suggestions

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.


As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

book.rb Outdated
@@ -0,0 +1,9 @@
class Book
attr_accessor :title, :author
Copy link

Choose a reason for hiding this comment

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

  • :rentals should also be in the getter and setter so that it could be read and set since we will be adding(setter) data based on some available(getter) data.

book.rb Outdated
Comment on lines 7 to 9
@rentals = []
end
end
Copy link

Choose a reason for hiding this comment

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

  • In the same vain, there should be an add_rental method here too for the person, it can look as follow:
def add_rental(person, date)
    Rental.new(date, self, person)
  end

Comment on lines 13 to 15
end

def of_age?
Copy link

Choose a reason for hiding this comment

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

  • As recommended for add_rental method in the book.rb file, it applies here too because it would be adding the book and it should do that itself too.

@RileyManda
Copy link
Owner Author

RileyManda commented Sep 4, 2023

I have completed the following requested changes on the School Library Application:

  • Implement add_rental method in Book class
  • Implement add_rental method in Person class

Please kindly review my implementation and suggest any changes and improvements.

I look forward to your feedback.

Thank you 🙏🏻

Best regards,

Copy link

@ichala ichala left a comment

Choose a reason for hiding this comment

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

Hi Student ,

Status: Approved ✔️ ✔️ ✔️

68747470733a2f2f6d65646961322e67697068792e636f6d2f6d656469612f6c3366516333614356304871707a5763552f67697068792e676966

To Highlight 💪🏻

  • Changes Implemented ✔️

Good job implementing the requirements 👍 Your project is complete! There is nothing else to say other than... it's time to merge it :shipit:
Congratulations! 🎉

Optional Suggestions :

  • N/A

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag ichala in your question so I can receive the notification.


As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

@RileyManda RileyManda merged commit 60302e6 into dev Sep 4, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants