Thanks for comment. I'll fix all problems and change everything you mentioned, hope to succeed in it.
Mark 2014-02-09 22:51 GMT+02:00 Pharo4Stef <pharo4s...@free.fr>: > Hi mark > > I read a bit your code > > It is nice for a first project!!!! > > Here are some feedback. I suggest that you take into account the easy one > and produce a new version and if you want to > train your skills you try to see how to address the difficult ones. > > Stef > > > > - Add it in the class comment > XsAndOsGame new openInWorld. > > - you have some unclassified protocols on the class side. > > - you may want to run smallLint (criticBorwser on your code and see the > hints that it can give you). > > - When you create a new game do not move the window to the top corner :) > > easyBtn -> easyButton (you do not pay per character :)) > > findEmptyCellPos -> findEmptyCellPosition > not in accessing > > impBtn -> impossibleButton > > .... :) > - Why do you need a separate class just for three colors? > I would merge > XsAndOsGameColors and XsAndOsGame > > XsAndOsGame>>initialize is long > May be you should extract some methods from it (methods that you can then > reuse for other tasks) > > > renderOContents > | delta offset x1 y1 x2 y2 rect | > delta := CellRenderer cellExtent - 1. > offset := self offsetWithinGridForm. > x1 := offset x + self oCorner x. > x2 := offset x + delta x - self oCorner x. > y1 := offset y + self oCorner y. > y2 := offset y + delta y - self oCorner y. > rect := Rectangle new setOrigin: x1 @ y1 corner: x2 @ y2. > self targetForm getCanvas > fillOval: rect > color: XsAndOsGameColors gameBoardBackgroundColor > borderWidth: 3 > borderColor: XsAndOsGameColors OColor > > What do you refer to CellRenderer direcly? self class ? > > > rendererFor: aCell > "VERY BAD IMPLEMENTATION OF THIS METHOD!!!" > > aCell class = EmptyCell > ifTrue: [ ^ CellRenderer ]. > aCell class = XCell > ifTrue: [ ^ XCellRenderer ]. > aCell class = OCell > ifTrue: [ ^ OCellRenderer ] > > indeed :) > why not implementing some methods call renderClass and sending a message > to the object :) > > mptyCell>>initialize > super initialize. > > > > CellRenderer>>offsetWithinGridForm > "Returns a point - left top corner of current cell" > > ^ CellRenderer cellExtent * ((self cellLocation x - 1) @ (self > cellLocation y - 1)) > => self clas cellExtent > > CellRenderer >> renderBorderBottom > | delta offset x1 y1 x2 y2 | > delta := CellRenderer cellExtent - 1. > offset := self offsetWithinGridForm. > x1 := offset x. > x2 := x1 + delta x. > y1 := offset y + delta y. > y2 := y1. > self targetForm getCanvas > line: x1 @ y1 > to: x2 @ y2 > width: 1 > color: XsAndOsGameColors cellBorderColor > > => > CellRenderer >> renderBorderBottom > | delta offset x1 y1 x2 y2 | > delta := self class cellExtent - 1. > offset := self offsetWithinGridForm. > x1 := offset x. > x2 := x1 + delta x. > y1 := offset y + delta y. > y2 := y1. > self targetForm getCanvas > line: x1 @ y1 > to: x2 @ y2 > width: 1 > color: XsAndOsGameColors cellBorderColor > > > I was wondering why you need Grid (I would rename it CellGrid) and do not > use a Matrix. > > grid sizeOfGrid > -> I would rename sizeOfGrid into size like that your grid can be a bit > more polymorphic from its API point of view > > > makeGameBoarMorph > -> typo :) > Now I want wondering why you do not have an instance variable to hold it > > > Future fun ideas for new versions: > -------------------------------------------- > playerVsComputer > self vsComputer > ifTrue: [ > self vsComputer: false. > self stateMorph contents: 'Player 1 VS Player 2' ] > ifFalse: [ > self vsComputer: true. > self stateMorph contents: 'Player VS Computer ---> Difficulty: ' , self > difficulty asString ]. > self vsComputer > ifTrue: [ > self easyBtn enable. > self normalBtn enable. > self impBtn enable ] > ifFalse: [ > self easyBtn disable. > self normalBtn disable. > self impBtn disable ] > This kind of code may be think that may be you need another object that > represent the > TwoUsers > vsComputer > strategy > I'm not sure but clicked look quite long. > May be you should create another class that can represent the logic of the > game. > > A really interesting exercise would be to see how you can create a > boardGame for 15*15 and see how it breaks some hidden rules. > On 08 Feb 2014, at 15:07, Маркіян Різун <mri...@gmail.com> wrote: > > Updated XsAndOsGame: added AI, also fixed some problems, that Damien > mentioned in his comments. > Btw, thanks to Damien, because I learned for myself a lot of new useful > things. > Note: The new version is XsAndOsGame (NOT Xs&OsGame) ! > To play it type in workspace: XsAndOsGame new openInWorld. > If you want to play vs computer press button "ToggleMode" and choose > difficulty. Initially it is easy. > > Mark > > > 2014-02-05 15:57 GMT+02:00 Маркіян Різун <mri...@gmail.com>: > >> Thanks, I'll certainly take a look at it >> >> >> 2014-02-05 Damien Cassou <damien.cas...@gmail.com>: >> >> On Tue, Feb 4, 2014 at 8:58 PM, Маркіян Різун <mri...@gmail.com> wrote: >>> > I should think how to write this tutorial. >>> >>> >>> on the technical point of view, I advise you to use Pillar: >>> http://www.smalltalkhub.com/#!/~Pier/Pillar >>> >>> -- >>> Damien Cassou >>> http://damiencassou.seasidehosting.st >>> >>> "Success is the ability to go from one failure to another without >>> losing enthusiasm." >>> Winston Churchill >>> >>> >> > >
