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

upload #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

RegaiszRoland
Copy link

No description provided.

Copy link
Contributor

@webmaster442 webmaster442 left a comment

Choose a reason for hiding this comment

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

Szép munka

}


public Polinom Clone()
Copy link
Contributor

Choose a reason for hiding this comment

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

Ez egy DeepClone implementáció, amit jó lenne ha a metódus neve jelezne.

{
while (Numbers.Count > 0 && Numbers[0] == 0)
{
Numbers.RemoveAt(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lista elejéről törlés piszok költséges. Stack esetleg? vagy Láncolt lista a sima lista helyett?

{
if (obj is Polinom other)
{
RemoveLeadingZeros();
Copy link
Contributor

Choose a reason for hiding this comment

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

Öö :D Az Equals módosítja az objektum belső állapotát a RemoveLeadingZeros() hívással?? Ennek nem így kellene működnie. Ha a RemoveLeadingZeros() szükséges, akkor azt miért nem a létrehozáskor hívod? Az Equals hívásnak NEM szabad módosítania belső állapotot. Ez nagyon durva Single responsibility és Liskov elv sértés is.

{
expression = Regex.Replace(expression, @"\s*([+\-*/%,()])\s*", "$1");

if (polynomials.ContainsKey(expression))
Copy link
Contributor

Choose a reason for hiding this comment

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

Ezt bőven szét lehetetett volna szedni egy külön osztályra.

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