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

Changing parameter for RenderLargeRectangles #100

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

Conversation

theilgaz
Copy link

@theilgaz theilgaz commented Jun 2, 2021

Changed Rectangle to Rectangle[] in Good example for Liskov Substitution Principle (LSP).

Changed Rectangle to Rectangle[] in Good example for Liskov Substitution Principle (LSP).
Copy link

@dmitryvhf dmitryvhf left a comment

Choose a reason for hiding this comment

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

Need more changes

@@ -2141,7 +2141,7 @@ class Square : ShapeBase
}
}

Drawable RenderLargeRectangles(Rectangle rectangles)
Drawable RenderLargeRectangles(Rectangle[] rectangles)

Choose a reason for hiding this comment

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

Please, make same into 2080 line, for "Bad" example

@dmitryvhf
Copy link

Conflict PR - #75

@theilgaz
Copy link
Author

theilgaz commented Jun 2, 2021

Fixed "Bad" example #101

README.md Outdated
@@ -1830,7 +1830,8 @@ class Employee

- [S: Single Responsibility Principle (SRP)](#single-responsibility-principle-srp)
- [O: Open/Closed Principle (OCP)](#openclosed-principle-ocp)
- [L: Liskov Substitution Principle (LSP)](#liskov-substitution-principle-lsp)
- [L:
v Substitution Principle (LSP)](#liskov-substitution-principle-lsp)
Copy link

@dmitryvhf dmitryvhf Jun 3, 2021

Choose a reason for hiding this comment

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

Excessed change. And new error.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I use search filter to find faster, but then I see error. I fixed this in here.

Copy link

@dmitryvhf dmitryvhf left a comment

Choose a reason for hiding this comment

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

Now correct

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