Closed MrSleazy closed 9 years ago
Hey @KevinSilvaQuintana
This looks good, but github is saying I can't auto-merge it which probably means you haven't merged in the latest code changes from this repo. If you do that, I'll merge this in. Also, I'd encourage you to take a look at the rewrite
branch...that's where we're doing most of our work now in an effort to improve the codebase from the ground up.
Hi Drew, sorry for the long reply. I have been busy with school. A team of software engineering majors and I have chosen your project for a refactoring project in a Software Architecture class. Unfortunately, our offered patchsets were done kind of in a hurry and do not really provide any meaningful improvements to your code. Had we had a lot more time, we would have tried to implement a strategy pattern to assign the authorized moving functions to each class (which you are currently handling with a LONG series of if statements). Thus, the code is really highly coupled and it became difficult for us to really dig into it in depth. Also, you should try to look into the use of interfaces and perhaps factories to make your code more readable. I will try to take a look at the rewrite branch (I think you have already begun some good refactoring practices). Anyhow, good job on the project it was very fun to work on it.
First patchset is concerned with the refactoring of the Piece class, which includes extracting movement methods from the genLegalDests(Board board) method. This way the cyclomatic complexity, number of paths and length metrics of the method get significantly reduced.
Second patchset is concerned with the elimination of duplicated code found in two methods of the Square Class. This was done by combining the two similar methods into a single one and adding a parameter to differentiate between them.