search

oezg / minesweeper-kt

Minesweeper game
0 stars 0 forks source link

Split Minesweeper responsibilities #2

Closed oscarnava closed 1 month ago

oscarnava commented 1 month ago

I see an issue with the Minesweeper class regarding the single responsibility principle. I think there should be another class (I suggest the name Grid) that deals with the Cells matrix, and is responsible for any related operations.

oezg commented 1 month ago

I think it is called the Model-View-Control. Grid shall be then Control?

oezg commented 1 month ago

Can you give me some hints on which responsibilities Grid should have as opposed to the ones that Minesweeper should have? Thank you.

oezg commented 1 month ago

What is your opinion about the latest version with Grid? Do you think a Cell should be aware of its location in the Grid or unaware. I always think it should be unaware to make the cell simpler but I am curious what you think.

oscarnava commented 1 month ago

I think it is called the Model-View-Control. Grid shall be then Control?

In that schema, from my POV it can be the Control and the View, considering that the View is quite simple here and there's no point in moving that responsibility anywhere else.

oscarnava commented 1 month ago

Can you give me some hints on which responsibilities Grid should have as opposed to the ones that Minesweeper should have? Thank you.

Sorry for not responding. My time zone is UTC-6:00, so if you are in Germany we have an 8-hour difference. 😬 Let me review the recent changes and I'll give you more feedback.

oscarnava commented 1 month ago

What is your opinion about the latest version with Grid? Do you think a Cell should be aware of its location in the Grid or unaware. I always think it should be unaware to make the cell simpler but I am curious what you think.

From working on CodinGame I've used many times that model of a Grid class and I would say the answer to your question depends on the problem. Sometimes you want the cell to be aware of its position, but I've found that that sometimes complicates things, as you implied. In general, when possible, a Cell without a location is a good approach, and you let the Grid take care of that (you can always pass that location to some Cell method, for example).

oezg commented 1 month ago

Thank you for your answer. I am trying to implement the other issues. I am aware of the time difference and please do not feel bad for not responding soon, it is perfectly OK for me. I already feel very lucky to have this conversation going with you. I can solve the project on my own but I appreciate your feedback to learn how to apply the principles of software development.