• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Bear Bibeault
  • Devaka Cooray
  • Liutauras Vilda
  • Jeanne Boyarsky
Sheriffs:
  • Knute Snortum
  • Junilu Lacar
  • paul wheaton
Saloon Keepers:
  • Ganesh Patekar
  • Frits Walraven
  • Tim Moores
  • Ron McLeod
  • Carey Brown
Bartenders:
  • Stephan van Hulst
  • salvin francis
  • Tim Holloway

My version of Conway's Game of Life  RSS feed

 
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Shouldn't a Cell know it's own Location within World? What smell does the design avoid by not giving Cell that responsibility?


I didn't try that approach, but that is one of the possible way to implement that. Will try. However, I don't think that in my mind before implementing something I look from the perspective: "what smell does that avoid by not implementing it that way", I think I look more like: "what I'd gain having that implemented". So probably at that time didn't see necessity of assigning Cell such responsibility to know its location. But as I said, I'll try this approach, it might prove to be a better design.

Junilu Lacar wrote:What is the flow of logic that calculates one generation to the next? It seems like the design was arrived at coming from a different perspective, one that was more speculative.


If understood question correctly, it calculates generation to generation while there are locations in a world which have alive cells, in different words, while world has population.
 
Sheriff
Posts: 12748
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Liu, I looked over the latest version of the code and I thought that it is very close to what it actually wants to be.

This Cell is still smelly.

I think this code wants to be:

See how it works here: https://repl.it/@jlacar/GoLCell
 
Junilu Lacar
Sheriff
Posts: 12748
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Regarding smells, they come from static Cell inNextGeneration(Cell current, int numberOfLivingNeighbors).

First of all, it's a non-private static method so that raises a small red flag. It's small because non-private static methods always make me suspicious but they can sometimes be justifiable to have. It becomes much smellier when I see Cell current as the first parameter. That's a really big red flag for me. These smells lead to an initial refactoring:

With this initial step of refactoring, the need for external code to pass in instances of Cell to the inNextGeneration() method is gone. External code just needs to invoke the next() method of the particular cell value it is dealing with.

That is, you can write this instead:

The next smells lead to the version I proposed. There are checks in the static method that look at specific values of the enum: current == ALIVE and current == DEAD. Those kinds of checks are smelly inside an enum static method. If there is logic specific to each value of an enum, then it's best to have the value-specific logic in a value-specific implementation of an abstract method. So, an abstract method is introduced and the value-specific logic is extracted appropriately.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:only things missing is something that sets the world to moving/ticking, and a colorful gui


Colourful GUI? It got even 2 colours! grey and orange Unfortunately, but I do not have design skills anywhere close to as Salvin has so you would need to pretend being joyful watching it.

I have very little experience with GUI's, so don't sneak in it (and I don't have mechanism yet to pass World's generation to GUI, for a quick experiment I just did via World.toString()). GUI is sitting in ui module's gui package. You can execute game by simply running its main class. You can't interact with system via GUI yourself (as of now), but for a start I've set a Glider pattern by hard coding it.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:This Cell is still smelly


Junilu Lacar wrote:I think this code wants to be


That's very nice! Like it a lot. Would feel bad stealing it, but very hard to resist.

Will analyse this implementation, not that I don't understand mechanics, but to understand rationale in this context and how it led you to this approach. But like it a lot. Thumb up.
 
Junilu Lacar
Sheriff
Posts: 12748
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Junilu Lacar wrote:What is the flow of logic that calculates one generation to the next? It seems like the design was arrived at coming from a different perspective, one that was more speculative.


If understood question correctly, it calculates generation to generation while there are locations in a world which have alive cells, in different words, while world has population.


Let me rephrase: It looks to me like the production code was written before the test code.

The test code looks like this:

Why are the tests only invoking a static method? Why aren't there any instance methods? When I do TDD, I don't usually test static methods very often and when I do, I don't usually test them directly. The one example that I can think of was when I was implementing succ() and pred() methods (successor and predecessor, respectively) of an enum class whose values had a specific ordering. I think I made these call a private static method, if I remember correctly.

I would shy away from a static call like Cell.inNextGeneration(Cell.ALIVE, 2). That's just not an API call that makes me think, "Yeah, that's what the code wants to do." I would think more along the lines of:
 
Junilu Lacar
Sheriff
Posts: 12748
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:Would feel bad stealing it, but very hard to resist.

Will analyse this implementation... to understand rationale in this context and how it led you to this approach.


I think as long as you don't just copy without understanding, it's not really "stealing" -- I would say "borrowing" is more like it if you understand the rationale behind the refactoring.
 
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Gerard Charles wrote:

Think for a moment how would you extend your system with more states than just alive/dead. I'm still bogged with this hypothetical future requirement


I'd most likely replace the boolean with an enum, depending on what the additional states turn out to be. A sometimes alternative is replacing a boolean with a Boolean (nullable, so it has three possible states) -- maybe if we have Schrodinger cat cells that are neither alive nor dead until queried? But dealing with nulls is often more trouble than its worth, so a enum is more likely.

 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:I like to think that l listen to the code. I like to think that the code tells me what it wants to be. The design tells me what it wants to be.


Really have to say that. I must admit that after reading that early in a morning I was thinking, how do you actually know what the code should be (note: with all the respect). You told me several times about the smell my Cell is carrying over. Then I was thinking even more, but how you can guarantee you know that's a smell, and other approach not smelling. Then after you showed refactored version of Cell.inNextGeneration() - it became apparent. I think when you see the well refactored version, you do not question it anymore, it becomes naturally clear what the design should be. Today was an eye-opener day which taught me a priceless lesson. My hat is off.
 
Junilu Lacar
Sheriff
Posts: 12748
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Junilu Lacar wrote:I like to think that l listen to the code. I like to think that the code tells me what it wants to be. The design tells me what it wants to be.


it becomes naturally clear what the design should be. Today was an eye-opener day which taught me a priceless lesson.


I'm glad I helped. I know it sounds kind of strange when I say things like "I listen to what the code and design want to be." Again, it's just another way for me to detach my "self" from the code and make it its own entity. After a while, the code seems to have a will of its own and I can "sense" whether or not I'm going with what it wants if it gets easier to work with and understand. If it gets more difficult, I take that as a sign that the code/design doesn't like the way I'm leading it.

I just finished a blog article about all this stuff. Check it out here: https://www.linkedin.com/pulse/game-life-lessons-static-smells-junilu-lacar
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks a lot to everyone who helped this thread going and reach the stage were we are now. I'm even more fascinated about this system now, such a small in terms of size, but fairly huge in the amount of lessons it has on offer.

Thanks for an article Junilu, it was very interesting to read the process of thoughts behind. Not the end, Strangler Pattern is awaiting to be read about.
 
Bartender
Posts: 1983
57
Eclipse IDE Google Web Toolkit Java
  • Likes 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Since someone mentioned that this does not look that colorful, here's a simple idea
Gol.jpg
[Thumbnail for Gol.jpg]
Let me know if you like it, I can share all assets pictures
 
Master Rancher
Posts: 3002
105
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Salvin,

you're the master... absolutely stunning!
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

salvin francis wrote:Since someone mentioned that this does not look that colorful


Oh, it is very nice! But this program doesn't work, I pressed Play, shows stuck screen    

Mine is minimalistic (see attached), which I like too, fits my taste. As I said, I'm not a designer, but I will try to come up with some other look.
GoL_liu.JPG
[Thumbnail for GoL_liu.JPG]
 
salvin francis
Bartender
Posts: 1983
57
Eclipse IDE Google Web Toolkit Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:..but I will try to come up with some other look.


okay, but, if you like my design, all you need is 3 pictures: a background (background.png), cell background (cellBack.png) and the actual Cell (cell.png).
See attached zip file.

Filename: gol-assets.zip
File size: 728 Kbytes
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Likes 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
But that's your design, right. I have my own taste and my own vision of look, that is the main reason why I want to have my own unique look rather than copy/paste somebody else's.

Thanks though.
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So I was curious at to what it would take to connect my JavaFX front end to this implementation. (See pull request on github.) Fairly straightforward, except a a couple tweaks:
* I needed a deadAt method to mirror aliveAt.
* Had to make some methods public to set / access the state of the World.

I'm am curious as to your thought process in the static Location.of instead of simply having a public Location(int rowIndex, int columnIndex) constructor?
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:I'm am curious as to your thought process in the static Location.of instead of simply having a public Location(int rowIndex, int columnIndex) constructor?


If you get a chance, read an Item 1 from Effective Java by Joshua Bloch. It has comprehensive chapter about that, advantanges and dis

However, my main reason was readability in this case. I really like that way, which seem to go in line what newer Java version follow as as naming convention, i.e.: List.of(...), Map.of(...).... Yes, new Location() is also fairly readable in this case.

One of other advantages could be, that I'm still left with an option to cache Location objects if I wanted to in a future, while habitual way to create object through constructor does not  give you such flexibility, whenever you write "new" you create a new object.

Don't want to list other advantages and repeat what is way better written in a book, but there are quite a few advantages over constructors.
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Gerard Charles wrote:I'm am curious as to your thought process in the static Location.of instead of simply having a public Location(int rowIndex, int columnIndex) constructor?


If you get a chance, read an Item 1 from Effective Java by Joshua Bloch. It has comprehensive chapter about that, advantanges and dis

However, my main reason was readability in this case. I really like that way, which seem to go in line what newer Java version follow as as naming convention, i.e.: List.of(...), Map.of(...).... Yes, new Location() is also fairly readable in this case.

One of other advantages could be, that I'm still left with an option to cache Location objects if I wanted to in a future, while habitual way to create object through constructor does not  give you such flexibility, whenever you write "new" you create a new object.

Don't want to list other advantages and repeat what is way better written in a book, but there are quite a few advantages over constructors.



Interesting read. (found a copy Effective Java). (Of course, we no longer have to specify all the types in generic creation, a development Bloch seemed to anticipate).

I'm just used to the standard Java library conventions (e.g. Point2D constructor), so that's what I'd expect to find in a "point" class.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:So I was curious at to what it would take to connect my JavaFX front end to this implementation. (See pull request on github.) Fairly straightforward, except a a couple tweaks:
* I needed a deadAt method to mirror aliveAt.
* Had to make some methods public to set / access the state of the World.


Thanks for taking this exercise. Some of the points which I do not agree with.

1 bug is introduced. deadAt() method is implemented wrongly. I don't record DEAD cells, I simply hold only ALIVE cells in a world map. So method deadAt(Location location) would really have to simply remove such ALIVE location from a map, that is way better memory management in my opinion.

Cell type. I had package private, now it got exposed as public. But I pressume for a reason of mentioned above. So really that is not needed with current design.

World.java Cell at(Location) as well as worldWrapped(Location) instead private became both public. For what reason?


What I think you'd need are at most 2 new methods, and nothing else should be changed.
1. deadAt(Location) so user could toggle, aliveAt/deadAt (you did that almost correctly already)
2. Map<Location, Cell> getWorldMap(), and that would be a copy of original, so front end couldn't mess up with world's state.

And that is it. Point [2] I currently mimicked with toString(), which is not the right way, but I didn't concentrate on a GUI yet.

The good thing about less public methods, less mutable states, that API becomes easier to use.

So really API currently contains only such methods (would be +2 methods from above):
World

Location


I need to analyze your GUI implementation (thanks), I have little experience with that.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:So I was curious at to what it would take to connect my JavaFX front end to this implementation.


You really deserved another cow for curiousity, so please accept, actually you can't refuse
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:
World.java Cell at(Location) as well as worldWrapped(Location) instead private became both public. For what reason?
...

2. Map<Location, Cell> getWorldMap(), and that would be a copy of original, so front end couldn't mess up with world's state.


The GUI intentionally messes with the world -- it allows the user to click on the grid and toggle cells on or off (apparently incorrectly with the incorrect deadAt).
It include's a drop down to include a couple gliders. The implementation of that needs to be able to add
locations modulo the grid, which is why the code is calling worldWrapped.

 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:It include's a drop down to include a couple gliders. The implementation of that needs to be able to add
locations modulo the grid, which is why the code is calling worldWrapped.


I don't think so. API does that already, you don't need to do that in a front-end. You just need to pass pattern's alive cells locations.

Gerard Charles wrote:The GUI intentionally messes with the world -- it allows the user to click on the grid and toggle cells on or off


Understand that, but since my nextGeneration() always construct a new world, for me it makes sense to have all in tact and don't rely on GUI implementor's vision. If GUI wants to change something, it can change through aliveAt(), deadAt(), that way I can assure that world is in consistent state. So getWorldMap() actually supposed to return immutable map.
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Gerard Charles wrote:It include's a drop down to include a couple gliders. The implementation of that needs to be able to add
locations modulo the grid, which is why the code is calling worldWrapped.


I don't think so. API does that already, you don't need to do that in a front-end. You just need to pass pattern's alive cells locations.

Gerard Charles wrote:The GUI intentionally messes with the world -- it allows the user to click on the grid and toggle cells on or off


Understand that, but since my nextGeneration() always construct a new world, for me it makes sense to have all in tact and don't rely on GUI implementor's vision. If GUI wants to change something, it can change through aliveAt(), deadAt(), that way I can assure that world is in consistent state. So getWorldMap() actually supposed to return immutable map.


Actually I didn't need a public worldWrapped since I added an "add" method, but that does need worldWrap, because if the GUI's calculated location of a point is negative (e.g. when placing a glider) the world API doesn't wrap the left side to the right (or top to bottom)-- actually fails silently due to the Math.abs in the implementation.



 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:Actually I didn't need a public worldWrapped since I added an "add" method, but that does need worldWrap, because if the GUI's calculated location of a point is negative (e.g. when placing a glider) the world API doesn't wrap the left side to the right (or top to bottom)-- actually fails silently due to the Math.abs in the implementation.


Not sure I understood fully, but I don't/wouldn't expect GUI to calculate anything, just display whatever is calculated and givent back by the system's nextGeneration() instruction.

But maybe you have bigger capabilities plan with your GUI so my API just not suitable for that. That would be also kind of normal and expected. Not something we never seen before.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:because if the GUI's calculated location of a point is negative


Wondering how that could happen? Does it work like x and y axis where 0 is in a center? My model follows rows, columns idea starting from the upper left corner where 0's are.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Let me check that more carefully, I might have bug how I let create Locations if initially they are with negative indices.
 
Piet Souris
Master Rancher
Posts: 3002
105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
We have arrived at a point where the discussion is hard to understand, if at all.

Anyway: what I was wondering: if incorporating some GUI makes it necessary to adjust the World code in places, then what is wrong with, or missing from, the design?
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:Let me check that more carefully, I might have bug how I let create Locations if initially they are with negative indices.


Yep, I did have a bug, the one Piet told me a month ago. I thought I fixed it..., it seems not. Bug was in particular scenario when location's abs(negativeIndex) > world size. Fixed this time.
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:We have arrived at a point where the discussion is hard to understand, if at all.

Anyway: what I was wondering: if incorporating some GUI makes it necessary to adjust the World code in places, then what is wrong with, or missing from, the design?



Well, probably easiest to run my proposed variant to see what the interface is doing first and then look at how GraphicDisplay is trying to allow CShape to place a pattern on the world.

But it wasn't necessary to adjust the World, nor is their necessarily anything wrong with the design -- I just playing around with merging two different designs. Personally I thought the question: Hey, world, what's the coordinates of the Location four to the left of this one? was a reasonable one to ask.
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Gerard Charles wrote:because if the GUI's calculated location of a point is negative


Wondering how that could happen? Does it work like x and y axis where 0 is in a center? My model follows rows, columns idea starting from the upper left corner where 0's are.


Well, it happens in:

some of those neighborLocation values will be negative, but because the code calls worldWrapped they become positive. Logically, I'd expect setAlive to do the same thing.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:some of those neighborLocation values will be negative, but because the code calls worldWrapped they become positive. Logically, I'd expect setAlive to do the same thing.


This is what it does.
 
Gerard Charles
Ranch Foreman
Posts: 39
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Gerard Charles wrote:some of those neighborLocation values will be negative, but because the code calls worldWrapped they become positive. Logically, I'd expect setAlive to do the same thing.


This is what it does.


My bad, was looking at the old version.
 
Liutauras Vilda
Marshal
Posts: 6263
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yet another experiment.

Inspired by the latest talks in Junilu's created topic about the Game of Life Lessons: Detecting Code Smells, I have committed a completely immutable system, meaning, system or its separate parts do not mutate in any way, nor directly, nor indirectly.
Staff note (Liutauras Vilda):

Test coverage dropped slightly, but will catch up.

 
Don't get me started about those stupid light bulbs.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!