-
Notifications
You must be signed in to change notification settings - Fork 29
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
add week0002.cs #70
base: master
Are you sure you want to change the base?
add week0002.cs #70
Conversation
var pointer = new Cell(0, 0); | ||
|
||
//Change direction points | ||
var MinMin = new Cell(1, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename:
MinMin => TopLeft
MinMax => TopRight
MaxMax => BottomRight
MaxMin => BottomLeft
} | ||
|
||
|
||
pointer += move; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are mixing up Cell
& Movement
. Although the code works, this does not make much sense from design perspective. Maybe you can simplify your code.
|
||
if (pointer.Equal(MinMin)) | ||
{ | ||
move = new Cell(0, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are moving Right
.
} | ||
else if (pointer.Equal(MinMax)) | ||
{ | ||
move = new Cell(1, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are moving Down
.
{ | ||
MaxMax.X--; MaxMax.Y--; | ||
} | ||
move = new Cell(0, -1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are moving Left
.
{ | ||
MaxMin.X--; MaxMin.Y++; | ||
} | ||
move = new Cell(-1, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are moving Up
.
|
||
for (var i = 1; i <= m * n; i++) | ||
{ | ||
array[(int)pointer.X][(int)pointer.Y] = i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's confusion: In your case X
represents row (vertical ordinate) and Y
represents column (horizontal ordinate).
It is counter-intuitive and hard to read. I would suggest to swap X & Y so they carry default meaning: Y for row & X for column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see you have a solution but it's a bit complicated for this simple problem and needs to be more intuitive.
Thank you for your review, I will improve my code. |
submit week2