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

Added Refactoring changes #11

Merged
merged 4 commits into from
Mar 24, 2022
Merged

Conversation

guryashdhall
Copy link
Contributor

Details of the Refactoring are present in the issue:- #9

@@ -15,6 +15,9 @@ The **Class Diagram** of the project is also provided along with the **Database
## Class Diagram
![class diagram](../master/images/diagram.PNG)

**Note**: After Refactoring, new Class "HoldRequestOperations" is added to the above structure which lies in between the HoldRequest class and Book class.
This class removes the bidirectional dependency between HoldRequest and Book.

Copy link
Member

Choose a reason for hiding this comment

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

@guryashdhall can we add why we made this new class and the context for this decision. Will be helpful for the readers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harismuneer sure i will update the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harismuneer I have updated the reasoning behind the introduction of new class in the README

@harismuneer
Copy link
Member

@guryashdhall I am merging the PR with the note that a subsequent code review from your peers would be required to ensure integrity.

@harismuneer harismuneer merged commit f3fd39a into OSSpk:master Mar 24, 2022
@amansbhandari
Copy link

@harismuneer I have reviewed the changes and tested the functionality , the changes made by @guryashdhall are just refactoring changes and has no effect on the working of the Library Management System

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

Successfully merging this pull request may close these issues.

3 participants