On Mon, Feb 3, 2014 at 11:39 AM, �������������� ���������� <mrizun@gmail.com> wrote:

As a novice in pharo, waiting for comments and advice.

��ritique is welcome too ;)

random comments (tested on Pharo 3.0):

- I played and things work perfectly as far as I saw :-)

- your code is of very good quality!

- 2 tests don't pass (EmptyCellTestCase, CellRendererTestCase)

- usually we don't append 'Case' at the end of our test classes

- the package name doesn't follow the convention (https://ci.inria.fr/pharo-contribution/job/PharoForTheEnterprise/ws/Conventions/Conventions.pier.html)

- you may want to avoid the use of non-alphanumeric characters in the package name (e.g., avoid &)

- please indent your code (use ALT+Shift+u on each method to get automatic indentation)
- "self should: [ offset = (50 @ 50) ]" --> "self assert: offset equals: 50@50"
- "self should: [ cell isOff ]." --> "self assert: cell isOff"

- all subclasses of Cell override #initialize in exactly the same way. Consider moving that to the superclass

- you should try to avoid as much as possible doing a super send in a different method. For example, EmptyCell>>initializeState does a super send on #initialize.

- OCell and XCell are exactly the same, this should not be and is very suspicious.

- CellRenderer has 2 methods with an uppercase letter. This is not conventional

- you use a lot of variables you use only once. Consider inlining them. For example in CellRenderer>>offsetWithinGridForm, all variables seem useless : "^ CellRenderer cellExtent * ((self cellLocation x - 1) @ (self cellLocation y - 1))"

- CellRenderer>>renderContents uses branching on the class of objects instead of polymorphism. That's bad design in my opinion. Consider delegating to OCell and XCell or using double-dispatch.

- Same thing for CellRenderer class>>rendererFor:

All in all, very good job. Would you consider writing a tutorial on how to implement an Xs&Os game in Pharo?

--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without losing enthusiasm."
Winston Churchill