Win a copy of Spring in Action (5th edition) this week in the Spring forum!
  • 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

Game of Life Lessons: Detecting Code Smells  RSS feed

 
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
One pair I worked with at Global Day of Coderetreat last Saturday had code like this:

That pair was actually working in Python but the code they had was essentially the same as that Java code.

My question to folks here is this: Would you see this code and think, "Oh, code smell..." or would you think it's a reasonable API?

I'd like to get some other opinions/perspectives before sharing what mine are.

Thanks!
 
Marshal
Posts: 61741
193
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What were the parameter names? Was the third willMethodWork?
 
Bartender
Posts: 9494
184
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I can deduce what the parameters mean without any further context. I wouldn't necessarily call it a code smell, but I would prefer (and I think the code would be clearer) if the first two parameters were encapsulated by a Location class.
 
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Would you see this code and think, "Oh, code smell..." or would you think it's a reasonable API?


With "reasonable" would be careful.

1. But an instant thing which bumped to me was, if the method name is setAlive(), what this boolean parameter might mean. And once such thought came to a mind, I think I can state it is a smell. Maybe knowing parameter name would clear things up, but it doesn't change the fact that such API call is un-intuitive/confusing/misleading.

2. Second thing to me is, those first 2 parameteres, (0, 0). Now I think most of us we'd relate that with coordinates, but that's because we have some ground knowledge about the game, and have some prior experience with it, but how are we certain they are coordinates? Couldn't it be setAlive(int cells, int monsters, boolean randomLocations)? Who knows! I have no evidence otherwise. Moreover, such functionality sounds reasonable as such. Having method name setAliveAt(0, 0), personally for me would make a huge difference (would add clarity). Seems subtle thing, but "at" gives some clues that it is being talked about the location. But again, I'm only speculating these two parameters mean coordinates, and if I catch myself that I need to speculate, that's something smelly.

3. Now that were mentioned location, I'd think why to pass these dry numbers instead of the Location, but that's probably not really a smell, but lack of abstraction (assuming they are coordinates), which in turn also breaks an encapsulation by exposing the coordinates system, while it is really not necessary to know the implementation details for the caller. By having Location, poor method's setAlive() name would read better without even improving its name to setAliveAt(), because Location would imply that idea that we are setting alive at Location.





 
Campbell Ritchie
Marshal
Posts: 61741
193
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This is beginning to sound like a very old advert for a Cola.

Somebody has told them about the beans pattern and they think you are supposed to have setAlive(true) and setAlive(false) rather than setAlive() and killOff(). Not that the last method name sounds very good.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you, gents. Some good stuff to discuss already but I'm glad to see the views expressed so far line up with my own.
 
Master Rancher
Posts: 3001
105
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No, I don't get any smell. Like Stephan, I think the meaning is clear, and if in doubt, I would look at the API of that method. It reminds me of the method BufferedImage.setRGB(x, y, rgb), and methods like setVisible(false) and setOpaque(true). Never had any problem with these.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ok, I think we have all the usual suspects in the room now so I'll share my thoughts.

While I agree that the method call seems more or less intuitive, I have been greatly influenced by Uncle Bob when it comes to boolean parameters. He seems to always treat those with suspicion and I think I have a good understanding why.

Consider these possible alternatives and how they might change the way you approach the design:

It looks like Campbell and Liu might like these alternatives but I'd like to hear what Stephan and Piet think.
 
Liutauras Vilda
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:... I think the meaning is clear, and if in doubt, I would look at the API of that method.


Disagree.

Let assume setAlive(true/false) means something what is clear to you.  Let's assume we sort of know what we are talking about, but doesn't ring the bell such method call:
setAlive(0, 0, false)

So what this false really tells us here?

1. That setAlive at coorindates 0, 0 won't be proceeded because we have false? (would sound like we passed coordinates just to figure out, that 3rd parameter says we won't use them)

or

2. That setAlive at coordinates 0, 0 will set something else than alive? What that something else is? Dead? Why Dead? Code doesn't suggest that either.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In particular, be mindful of where you're translating values inside your head versus saying something explicitly with code.

For example, when you say "Well, false just means it's not alive," think of where that translation happens. What if someone thinks differently or has different experiences? Would they come to the same conclusion or make the same translation? How do we know that "false" or "true" value doesn't mean "don't count non-existent cells as neighbors."

Did your knowledge of the problem or any similar past experience influence your interpretation of the code? What if it didn't turn out to be designed that way? Would you have bothered to actually look at the implementation to see if your assumptions were correct?

I think these have implications on how bugs can get introduced into our code.
 
Piet Souris
Master Rancher
Posts: 3001
105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Is also very clear.

But then the question is: what does 'world.at(x, y)' return that can be set to alive or dead, and why do i need to know that? The first method 'world.setlive(x, y, false)' hides the in-between 'at' method. So, it still makes sense to me. What would I choose, if you would ask me? Hmm, good question. Like Salvin, I used a structure such that a Cell/Location is alive IIF it is part of that structure. If I had to use a structure as implied here, I'd have to think about it.

 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Liutauras seems to have caught on to my wavelength here. Those are exactly the kind of questions I was referring to in my last reply.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:But then the question is: what does 'world.at(x, y)' return that can be set to alive or dead, and why do i need to know that? The first method 'world.setlive(x, y, false)' hides the in-between 'at' method. So, it still makes sense to me.


Think about this: which version probably has better separation of concerns?

Assume both have the same outcome.
 
Piet Souris
Master Rancher
Posts: 3001
105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:Disagree. (...)


Allright, but I must say your arguments are a little far-fetched. As I mentioned, when I have a JLabel j, and do j.setOpque(false), then what would you conclude? Would you have the same arguments?
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:

Liutauras Vilda wrote:Disagree. (...)


Allright, but I must say your arguments are a little far-fetched. As I mentioned, when I have a JLabel j, and do j.setOpque(false), then what would you conclude? Would you have the same arguments?



But let me suggest that your past experience is talking here, not so much the code. We have to let the code talk to us. We have to make it tell a story, and a clear story because not everyone might have the same past experiences.
 
Liutauras Vilda
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:

Liutauras Vilda wrote:Disagree. (...)


Allright, but I must say your arguments are a little far-fetched. As I mentioned, when I have a JLabel j, and do j.setOpque(false), then what would you conclude? Would you have the same arguments?


I see your point. But since your examples are GUI elements, I'd feel safer with assumptions, because GUI shouldn't contain some business logic behind apart from the facts, on/off/, yes/no, white/black, size(1/2/3...).

While actual API is about the existing design, which might be different from what you think.
 
Piet Souris
Master Rancher
Posts: 3001
105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Piet Souris wrote:But then the question is: what does 'world.at(x, y)' return that can be set to alive or dead, and why do i need to know that? The first method 'world.setlive(x, y, false)' hides the in-between 'at' method. So, it still makes sense to me.


Think about this: which version probably has better separation of concerns: world.at(x, y).setDead() or world.setAlive(0, 0, false)? Assume both have the same outcomes.


I really don't know. I'm just experimenting with a model and a controller, with an interface, that among them defines the data structure used in the in-between traffic. But the World (seen as model) may internally use any representation that it wants. In that view 'world.setAlive(x, y, boolean) might make sense, hiding enough details.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Consider other alternatives though:

Aren't these semantics clearer and more explicit?
 
Liutauras Vilda
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My preference would be more like this, which minimizes ambiguities about the numbers, even though ambiguities aren't big. But again, as been mentioned before, I think we feel so confident about the parameters only because all of us implemented this game, so we know well what's in a system and what's not.
 
Piet Souris
Master Rancher
Posts: 3001
105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes they are.

But the original line of code was for me also clear. So, I hope that this topic will make clear in how far some objective measure exists that determines whether some code is smelly, or that it in general is about personal feelings and (in your case: a huge) level of experience.
 
Liutauras Vilda
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Piet Souris wrote:(in your case: a huge) level of experience


I know you are about Junilu here, but that's the case about you too. Your experience let relate some Jlabel examples with these method calls, so you re-assembled what they mean right away + you knowing the GoL was an advantage too.

Now if some grad who just started gets task to extend it, seeing setAlive(0,0, false) quite likely would have to dive to a method's signature to clarify what parameters mean, while other calls probably would make him/her (including me) feel way more confident, and that's only because we don't have so much experience with other API's where such methods are habitual.
 
Piet Souris
Master Rancher
Posts: 3001
105
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:

Piet Souris wrote:(in your case: a huge) level of experience


I know you are about Junilu here, but that's the case about you too. Your experience let relate some Jlabel examples with these method calls, so you re-assembled what they mean right away + you knowing the GoL was an advantage too.

Now if some grad who just started gets task to extend it, seeing setAlive(0,0, false) quite likely would have to dive to a method's signature to clarify what parameters mean, while other calls probably would make him/her (including me) feel way more confident, and that's only because we don't have so much experience with other API's where such methods are habitual.


I'll quote the whole reply, so to be sure to what post I am replying.

You certainly have a good point, I won't deny it. But the question was if one signalled a code smell, and for me that wasn't the case. And another goal was to get some real discussion, for there is the danger that whatever Junilu writes (with his enormous reputation), we take for granted and don't dare to doubt it (and of course: I mean this as a compliment).

So: up to the next example!
 
Liutauras Vilda
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
One thought came to my mind how to compare such methods:
and

In literature there are such terminology as "open-ended questions" and "close-ended questions".

So first example I see as "close-ended question" series. Do you want alive cell at indices x, y or not? Answer: yes/no
Two latter examples are more of "open-ended question" series. They require you to explain more clearly what you answer is and why.

In literature lessons I remember it used to be, that teachers used to ask "open questions", so the dialog would keep going, the rationale behind the answers would be clarified. While the "close-ended answers" were considered as simply pupil didn't invest much time in analysing given interpretation text (in this case system), so answers are laid down in a yes/no fashion which might be guessed correctly.

May sound stupid, but that's really how I see it now. Quickest way to answer the question how to set alive or dead cell, is just answer setAlive(0, 0, true/false). Not expressive.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I love hearing counterarguments because they make me think about different perspectives.

I think back to my "self" a number of years ago and I probably would have thought like Piet.

Getting sensitive to smells is a long and personal journey. To some extent, there is an element of personal taste as well but I try to keep my reasoning grounded in principles. The rules that I feel are violated by the original version are expressiveness and clarity of intent and leaky abstraction.

Leaky abstraction because the fact that the ideas of "alive vs dead" has now been exposed as a boolean in the first version. When you have just setAlive() and setDead(), no implementation detail is leaked.
 
Campbell Ritchie
Marshal
Posts: 61741
193
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:. . . all the usual suspects . . .

Hahahahahahahahahahaha!

It looks like Campbell and Liu might like these alternatives but I'd like to hear what Stephan and Piet think.

I haven't suggested a favourite yet.

At first I thought setAliveAt(x, y) and setDeadAt(x, y) looked good. But I am no longer sure. I don't think I like the construct with all the dots in because that isn't intuitive to write.
Now I am going to disagree with Liutauras, I think. The concept of the game is to have squares which contain living organisms or don't contain them. There is no notion of setting a square alive, but of reproducing into it and filling it with a living organism. Though these poor critters do appear to have a short lifetime. So, should we say something like organism.reproduceInto(x, y), or organism.die()?
 
Junilu Lacar
Sheriff
Posts: 12747
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:
In literature there are such terminology as "open-ended questions" and "close-ended questions".
...
May sound stupid, but that's really how I see it now.


No, not stupid at all. Interesting, definitely. I never thought of looking at it that way and even when I try, I'm not quite making the connection. I know what open-ended and close-ended questions are though. I'm wondering whether this a function of how you think in your native language versus in English. I have heard that certain cultures are better at math because of the way they talk about numbers. People who speak languages that use monosyllabic words for numbers and don't use decimal place positioning words like "hundred" or "thousand" tend to be better at math, or so they say. For example, in Chinese, $29 is said pretty much the same way as $2.90 (I think), and the context will determine what the meaning of the same words are. They just say "two-nine" and whoever you're talking to has to insert the decimal point where it is appropriate.

I wonder if there's something similar going on with the way you think of these different versions as being similar to open-ended and close-ended questions.
 
Campbell Ritchie
Marshal
Posts: 61741
193
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
But I suppose organism.die() would have to be followed by organism.decompose()
Otherwise how would you remove the reference to the organism from the square? You have a grid with comprises squares (composition) and each square may contain an organism, which is more like association. I can think of a few ways to remove an organism.
 
Liutauras Vilda
Marshal
Posts: 6257
420
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:I wonder if there's something similar going on with the way you think of these different versions as being similar to open-ended and close-ended questions.


First, I've been less than optimum in literature, I think I failed exam with my first attempt.

But to simplify what I wanted to express with such relation, that initial version:
world.setAlive(0, 0, true) relates to a closed-ended answer, which not expresses answer throughout when is asked by the system where to set cell and whether it should be alive or dead. Just dry set of answers dropped as arguments without any logical sequence where one complement each other.

while improved:
world.at(location).setAlive() answers that world at certain location supposed to be alive. That re-assembles as a well formed sentence.

But ok, that is probably exactly same what you are telling always, just wrapped in a literature context. Thought will write such thought.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:But I suppose organism.die() would have to be followed by organism.decompose()
Otherwise how would you remove the reference to the organism from the square? You have a grid with comprises squares (composition) and each square may contain an organism, which is more like association. I can think of a few ways to remove an organism.


I think you're getting carried away with the metaphor.

I'm going to switch to Liu's reified version: world.at(location).setAlive() because I think it leads to that Cell enum we were discussing in his other GoL thread.

I can imagine how the at(Location) method might return a single-cell World. I can see that calling setAlive() on a World turns every cell in it as "alive" and that when called on a single cell World, you would just make that one cell alive.

This actually presents a great idea to experiment with and that idea of an enum Cell that Liu came up with in his other thread would come in very handy.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:I can imagine how the at(Location) method might return a single-cell World. I can see that calling setAlive() on a World turns every cell in it as "alive" and that when called on a single cell World, you would just make that one cell alive.


That's going to be a fractal World. Lots of new possibilities.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This is what I was getting at when I asked earlier how might this kind of alternative change the way you approach the design.
 
Ranch Foreman
Posts: 39
8
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:One pair I worked with at Global Day of Coderetreat last Saturday had code like this:

That pair was actually working in Python but the code they had was essentially the same as that Java code.

My question to folks here is this: Would you see this code and think, "Oh, code smell..." or would you think it's a reasonable API?

I'd like to get some other opinions/perspectives before sharing what mine are.

Thanks!



Nope, doesn't smell at all.

Code can't "smell." It's conceptual, inorganic text that doesn't emit molecules. As near as I can tell, "smell" means "I don't like it / I wouldn't have done it this way." It's an opinionated, pejorative term that tells me the speaker is unable or unwilling to communicate in a professional manner.

As far as the method call, from looking at it, I have  pretty good idea what it's going to do. It mirrors the pattern found in the JDK graphics API (e.g. fillText("Cow", 42,54). If I have any doubt, I look at the declaration, because that's where functions are described in Java. Good IDEs will jump you there quickly and/or show the API in a hover. JetBrain's Intellij will fill in the parameter names for you.
apifill.png
[Thumbnail for apifill.png]
JetBrain's filling in parameter names
 
Sheriff
Posts: 5446
147
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote: As near as I can tell, "smell" means "I don't like it / I wouldn't have done it this way."  


My opinion is that code smell is closer to, "I've seen this kind of code before and it usually doesn't end up well."  It's a little like the saying I learned early on in my coding career:

   When in danger, run away
   When in doubt, quit
 
Junilu Lacar
Sheriff
Posts: 12747
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

Gerard Charles wrote:
Nope, doesn't smell at all.

Code can't "smell." It's conceptual, inorganic text that doesn't emit molecules. As near as I can tell, "smell" means "I don't like it / I wouldn't have done it this way." It's an opinionated, pejorative term that tells me the speaker is unable or unwilling to communicate in a professional manner.


No, that's not it at all. Maybe you aren't familiar with the term and how it's used in quite a large section of the developer population. It's not pejorative at all, not in the sense that you interpret it.

The term comes from the Extreme Programming community. It was coined and popularized by Kent Beck, the co-creator of JUnit. As such, I hope you don't think that someone like Kent Beck is the penultimate example of a person who is unable or unwilling to communicate in a professional manner.

The story is that when Kent was a young father with little kids in diapers, he asked his mother how he would know when to change them. Grandma Beck reportedly answered, "If it stinks, change it!" Kent thought that was a great metaphor for code that had some kind of design flaw. He had done a lot of work with Martin Fowler, who wrote the book, "Refactoring." Kent suggested that problems in code that needed to be addressed and refactored be called "code smells" and Martin Fowler liked it. The original "Refactoring" book had 22 code smells listed in it. It also has 72 refactorings.

So before you accuse people of being such and such and make assumptions about their character, perhaps it would be prudent to ask a few probing questions instead to get the whole story.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Gerard Charles,

In case you missed it, most of us involved in this discussion have agreed to practice and adhere to the precepts of egoless programming. That means that whatever criticism is given about code, it is not meant to impinge on the ability, competence, or intelligence of whoever wrote it. We all have to accept that we don't write the best code all the time. Sometimes designs that we come up with have problems. That can be due to many factors but we don't care much about what they are. We treat the design as if it were its own entity. It has to be able to stand on its own merits or fall for the lack of it. No design choice is sacred. That's the only way we can improve our design and our choices. We have to detach our "selves" and egos from the code that we write.

With that said,

Gerard Charles wrote:
setAlive(row: 3, column: 4, value: true)


The hints that IntelliJ IDEA gives for the parameters are certainly helpful in many cases. However, in many instances, that feature can also be a dangerous crutch that makes your design muscles atrophy. Your IntelliJ IDEA example shows exactly why that design is smelly. The name "value" does nothing to clarify the intent of true or false. The meaning of true or false is in someone's head. It isn't expressed explicitly in the code.

Whereas a design like that which Liutauras has been proposing does:  world.setAlive(atLocation) and world.setDead(atLocation) leaves absolutely nothing to the imagination. Every bit of information needed to understand what the intent of the code is right there expressed in the code itself.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
We are going to be quite fortunate to have author and GDCR co-founder Corey Haines as our guest author next week. If you haven't already read his book, "Understanding the Four Rules of Simple Design," you can participate in the book promo discussion next week and get a chance to win one of four copies he's going to give away.

One of the sections in that book is exactly about this particular design choice.

Here's a preview of some code from the book (Corey uses Ruby examples):

So you see, it's not just "my preference."  There are very specific reasons for choosing this kind of design and Corey does a really good job at explaining what those reasons are. It's all related to rule #2 of Kent Beck's 4 Rules of Simple Design: The code must clearly express its intent.

The same section of the book talks about reifying the two parameters into a Location object that encapsulates the x and y values since those two are always going to used together anyway. Again, there is a great discussion in the book about the choice to reify x and y into a Location class.

This should make Liu feel good about his development as a software crafter because it's in line with Corey's example. If ever there's someone whom I can say is an authority in design and design principles, Corey Haines has to be right up there, along with the likes of Uncle Bob, Kent Beck, Martin Fowler, Michael Feathers, and Joshua Kerievsky, to name just a few in that good company of world-class developers and experts in software design and design principles.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Further challenge: think about why Corey's example has no set_dead_at(x, y) or eventually, after reifying x and y into a Location class, a set_dead_at(location) method.
 
Junilu Lacar
Sheriff
Posts: 12747
210
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Also, what might be a way to design it such that either of these is possible (I'm thinking about creating a fluent API):

Again, when I look at that code, a nugget of an idea that suggests "fractal" starts to form in my head.
 
Campbell Ritchie
Marshal
Posts: 61741
193
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:. . . I think you're getting carried away with the metaphor.

Is that why you called me suspect yesterday?

I'm going to switch to Liu's reified version: world.at(location).setAlive() . . . .

That would probably be the best of the remainder.

I am intrigued about how you would implement world.setAlive().at(Location).
 
Stephan van Hulst
Bartender
Posts: 9494
184
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:
It looks like Campbell and Liu might like these alternatives but I'd like to hear what Stephan and Piet think.


I've tried this approach in the past. I'm a big fan of how fluid it is, but in the end the amount of extra bookkeeping my code has to perform almost always outweighs the benefits, so I've dropped it in all my new projects in preference of using locations that don't have to know about the enclosing world.

The biggest drawback is that locations that have a reference to their enclosing world can not be compared to locations that come from a different world. Here are some confusing scenarios:

The general advice here is, don't create classes that are responsible for mutating other classes through side effects. Fluent interfaces work fantastically with immutable types and objects that only mutate themselves (builders).

The only exception I can immediately think of are 'view' objects, that are mutated when mutating the object that they are a view of. But that's exactly the point of them. I wouldn't say that a World counts as an aggregated view of Locations.
 
Consider Paul's rocket mass heater.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!