• 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 Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Have an object communicate with another object

 
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm sure many of you know Eric Lippert's Blog series: Wizards and Warriors. In it, he has two rules:

   A warrior can only use a sword.
   A wizard can only use a staff.

Over the series of blogs, he tries to show you how the C# type system fails to implement those rules and what to do about it instead. He suggest the creation of a Rule class, and let the Rule system determine what can happen in the game. You can agree or disagree, that's not the point of my question. I included it because it relates to my bigger question, how would you tell the Rule class or system that the object you're trying to carry is in fact a Sword without instanceof?

Before anyone accuses me of not trying, I did on three separate times.

My first attempt was to have each weapon contain an enum. This approach was considered bad,

Quote:

When your weapon types enum just mirrors the class hierarchy, then this is a blatant violation of the DRY principle - you encode the same information, the type of a weapon redundantly in two places - one place is the class name itself, one place the enum. That's definitely not just a code smell, that is bad code.




My second attempt was to introduce a private collection with attributes that any class attempting to use the Weapon can query using a method. This too was seen as complicating the issue.

Quote:

Without knowing more of your specific case, I would say yes, you are doing the same thing, you are just making it more complex.

The fundamental problem you are having, is that you are trying to decide how an object would react to some action from outside the object. This is fundamentally not how object-orientation works (or should work). You should ask the object directly to perform the action, and the object will hopefully tell you what the result is if any.



My third attempt, didn't even generate any answers.

This Game Dev link states something I agree with:

In terms of the comparison there isn't really getting around using an enum or something similar. Either the weapon or the thing adding the weapon needs to know if it can be added/equipped, and the only way to do that is to compare it to something known ahead of time and determine if it is true or false. Your options are rather limited in that area, if you don't compare against a weapon type then you'll have to compare against the item i'd or if it implements an interface or something else. These all equate to identifying what the thing is.



Problem:

Weapon is rightfully an abstract class.


Question:

Can someone show me how to implement the rule class and use it, so that when I need to check if a certain character can use a weapon, I can determine what weapon is being checked?

Or in others words...

Because Weapon is an abstract class, you can't determine exactly what weapon it is, so how do you communicate to the Rule class that the weapon you have is in fact a Sword and that a Wizard cannot use it.

Just because I'm a nice person, I'll put it another way...

The Rule class needs to enforce that a Wizard cannot use a Sword, but it will get passed a Weapon object which is an abstract class. How can the Weapon class communicate with the Rule class that it's a Sword?  

Sorry if this post comes off as rude, but I've been frustrated trying to figure this out, and the answers on SESE aren't helpful, they simple confirm something is a bad idea, but no solution on how to fix it.
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This is just a knee-jerk response but my first thought is double dispatch polymorphism. You use the language's ability to route polymorphic calls to the object/method that most closely matches the runtime type of the parameters given.

You might also want to search for Visitor Pattern Double Dispatch Java -- I'm pretty sure this is the pattern I would try to apply to this kind of problem.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
... and Welcome to the Ranch!
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Code-wise, I'd imagine you'd have a Wizard or Rule that looks something like this:


This uses the language's type system and polymorphic features to route calls like this properly:
 
Marshal
Posts: 79153
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Again, welcome to the Ranch

Junilu, would that entail throwing an exception from the method with the wrong parameter? Would yoiu throw checked exceptions only?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I probably wouldn't throw an exception. I can easily see a user try to give a Wizard a Sword to use. While you might try to avoid allowing them to do that in the user interface, the code still needs to be called to attempt it, or at least check a Rule object if it's allowed. I wouldn't throw an exception in that case.  This is just off the top of my head, mind you but I'd imagine if I were to write a Rule object, I'd have something like this:

and something similar for a WarriorRule.
 
Sveta Rosemond
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Code-wise, I'd imagine you'd have a Wizard or Rule that looks something like this:


This uses the language's type system and polymorphic features to route calls like this properly:



The issue is, you'll never deal with a Staff directly in this way. Staff will be stored in the Character inventory as a Weapon, so imagine that Wizard is a subclass of Character with a weapon inventory.

Example:



The problem is in the tryAdd() method, how can I communicate with the Rule class that I have a Sword or a Staff when I'm being given a generic Weapon?  Let's make it easier, don't worry about the Rule class for now, how would I determine in the tryAdd() method what weapon I have?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sveta Rosemond wrote:Let's make it easier, don't worry about the Rule class for now, how would I determine in the tryAdd() method what weapon I have?


That is precisely the problem solved by the Visitor Pattern Double Dispatch thing I referenced earlier. You the programmer WILL NOT determine what weapon is passed, the Java type system and polymorphism mechanisms will do it for you.

See the example for Visitor Pattern in Java: https://en.wikipedia.org/wiki/Visitor_pattern#Java_example

This is the pattern I would try to apply to this problem. I would probably change visit() to some other name, like canUse() or something like that and make it a boolean. For that matter, I probably not use the "Visitor" name at all and just use a name like WeaponRule.

So, you'd have


This is just off the top of my head. There may be some implementation details I'm not seeing right now (I'd have to try it in real code) but it seems conceptually feasible.

EDIT: The more I look at this, the more I think it's the way to go. By defining a WeaponsRule class like that, adding new weapon types is not that difficult. You add a new method for that weapon type to the WeaponRule class, then go to each Character type that is allowed to use that weapon and override the new method. Defaulting every canUse() method to return false just makes that particular type of weapon unusable by any Character until you explicitly go and override it in the characters who are actually allowed to use it. That way, you're only making the minimum number of changes and they're the ones that you will most logically want to make anyway.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Note that code I gave is just illustrating a concept. I can already think of several ways to refactor that code to make it better organized and ready to be easily modified in the future, should the need arise.

EDIT: I was able to apply Visitor Pattern double dispatch to this problem. See my code here: https://repl.it/@jlacar/VisitorPatternDoubleDispatch
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I usually advise people to make the code expressive enough so you don't need comments. However, in cases like this, expressive code isn't going to be enough to guide other developers on how to maintain this class. I would do something like this:

 
Saloon Keeper
Posts: 15488
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I recommend using an interface over a class here. Also, does it make sense to say that a Warrior is-a WeaponRule?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:I recommend using an interface over a class here. Also, does it make sense to say that a Warrior is-a WeaponRule?


I don't think I suggested that Warrior is a WeaponRule at all and certainly, no, it doesn't make sense to say that. The first two method signatures you suggested are a bit smelly to me though since they move the responsibility of determining the type and appropriate method to call back to the programmer. The type system and polymorphism should take care of that for you. The other two methods are better in that the type system and polymorphism are is used but you don't take advantage of polymorphism and the names redundantly include the type name. Whenever a name includes a specific type, it's almost always a code smell that needs to be refactored.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I think an interface with default methods is a good alternative choice to a class. You can still create anonymous classes from it for each character type.

I wasn't going to actually code it out but now I think there has been enough discussion to warrant putting the money where the mouth is.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Well, it took longer than I thought it would and it seems some of my assumptions on which I based the code I previously gave were slightly off.

I was correct in that you can apply the Visitor Pattern and double dispatch to leverage Java's type system and polymorphism to take care of details for you. There was more coding than I described above and some details are different in the implementation that worked.

To see the money, see this code: https://repl.it/@jlacar/VisitorPatternDoubleDispatch

I tried adding new weapons types and character types and it was pretty straightforward. Codifying rules like "Batman can't accept a Gun" or "Soldier is Warrior who can accept a Staff" seemed logical and more or less intuitive. I'd still probably add some developer comments in the code to guide maintenance programmers on how to extend and add new rules.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sveta Rosemond wrote:
Over the series of blogs, he tries to show you how the C# type system fails to implement those rules and what to do about it instead. He suggest the creation of a Rule class, and let the Rule system determine what can happen in the game. You can agree or disagree, that's not the point of my question. I included it because it relates to my bigger question, how would you tell the Rule class or system that the object you're trying to carry is in fact a Sword without instanceof?


As it turned out with the way I applied the Visitor Pattern, I created a WeaponRule interface with default methods that defined the rules for each different Character type.

The defaults say that neither Wizard nor Warrior is able to use a Weapon unless specifically allowed to by that Weapon subclass. Weapons are defined as such:

When you add a new Character subclass, you have to add another WeaponRule method for that subclass. Then you have to go through your Weapon subclasses and override the new method as appropriate.  I would use a default that applies to most Character subtypes and override the behavior in Weapon subclasses that are the exception to that default rule. That keeps the changes you have to make and keep track of to a minimum.

 
Sveta Rosemond
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
What I'm failing to understand is, why can't the Rule class simple use instanceof and check the Character and Weapon types and decided what to do?  

To my anyways, it seems like a lot of code for something that seems to simple. I'm not saying you're wrong, you obviously know what you're doing, but as you pointed out, using the double dispatch with the visitor pattern might be hard to maintain as more character types and weapons get added.  
 
Bartender
Posts: 2911
150
Google Web Toolkit Eclipse IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Just wanted to share what popped into my mind :

Feel free to criticize
 
salvin francis
Bartender
Posts: 2911
150
Google Web Toolkit Eclipse IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

salvin francis wrote:
Feel free to criticize


I just read that OP mention the DRY principle... My code looks horrible now
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sveta Rosemond wrote:What I'm failing to understand is, why can't the Rule class simple use instanceof and check the Character and Weapon types and decided what to do?


Using instanceof in most cases is a code smell that indicates you're not using polymorphism correctly. It's also inefficient since instanceof is a relatively costly/costlier operation, not that you should really be thinking about optimization prematurely but there's still that consideration.


I'm not saying you're wrong, you obviously know what you're doing, but as you pointed out, using the double dispatch with the visitor pattern might be hard to maintain as more character types and weapons get added.


Only if you're not careful and fail to maintain code hygiene, i.e. code cleanliness.  That's why I cited this as an exception to my rule of thumb to prefer expressive code over extensive comments. If this were production code, I'd leave clear and unambiguous comments for future maintenance developers to follow strictly.

I actually think that maintaining code hygiene is easier with this pattern in place since the points of change are known and well-defined. As long as maintenance programmers understand the pattern and how to extend it and they maintain discipline and hygiene in doing so, it should be pretty straightforward to add new rules and Weapon or Character subclasses to the mix. And you don't end up writing a whole mess of if-else statements. That's really what you're trying to avoid here. Instead of more-or-less hard-coded conditional logic, you use the type system and polymorphism to do the job for you. This results in code that has clearer and cleaner semantics.

And of course, there's that violation of the DRY principle that Salvin reminded himself of.  Using instanceof in this context is just another way of violating DRY, although it's a subtler more nuanced violation.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

salvin francis wrote:Just wanted to share what popped into my mind :

Feel free to criticize


Ok, I'll be yer huckleberry...

If this kind of idea ever pops into your mind, please pop it right back out. This is the kind of hard-coded conditional logic I was referring to that you want to avoid. It's just not good. Sorry, I'll have to dig around to find ways to explain why it's not good. All I can say right now is that it leads to more pain and confusion when you start adding new rules and subclasses of Character and Weapon.

EDIT:

salvin francis wrote:I just read that OP mention the DRY principle... My code looks horrible now


Oh yeah, there's that reason that OP cited.

Don't feel too bad. I've seen lots worse than that. There's a reason I lost my hair, you know, and it's not entirely because of MPB (unless you take that acronym to stand for "Maintaining Programs that were Bad")
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sveta Rosemond wrote:To my anyways, it seems like a lot of code for something that seems to simple.


Another thought about this: One of the most influential books in my career as a software developer is Martin Fowler's Refactoring: Improving the Design of Existing Code. The first chapter is a long refactoring example. It takes what appears to be a short and simple piece of code and improves its design through a series of disciplined and purposeful transformations. The result is longer than the original code and on the surface, seems to be more complicated. However, if you study it carefully, the code is actually better organized and well-structured.

While shorter code tends to be simpler and easier to work with, you need to look at the overall design and organization and weigh the benefits of structuring the code in certain ways that, while it may result in more code that seems more complicated, actually makes it easier to test, maintain, and extend.

I'm pretty sure that first chapter in the Refactoring book is available for download as a PDF. I encourage you to read and study it carefully because it can teach you a lot, as it did for me when I first read it.

Another book I'd highly recommend is Understanding the Four Rules of Simple Design by Corey Haines. This book only discusses one problem: Conway's Game of Life. However, it is full of valuable insights about design, design principles, and simplicity.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
[Shameless self-promotion ahead]

Here's another example of seemingly simple code that benefitted from refactoring. https://coderanch.com/forums/posts/preList/699995/3283755#3283755

The refactored code is longer than the original but at a high level, it's clearer and easier to understand. Since multiple concerns are separated into their own methods, the code is better organized and composed of smaller, more focused parts.
 
Sveta Rosemond
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Would an attribute system be a possible solution as well? I'm not saying you code or any of the answers posted are wrong. I just want to look at the problem from a different perspective and find out the different ways to solve it.

Take for example this code snippet from Clean Code, Pg 19:


From this code snippet, we know a know that a cell contains a method that verifies if it's been flagged. In other words, we can ask the cell, are you flagged?

We then collect a List of flagged cells and return to the caller.

With that logic and example in mind, we can have a GameObjectAttributes type, and a  Weapon class that looks like:




A Wizard class might look like this:


Much like the Clean Code snippet, I'm asking the object, do you contain a specific attribute, and if not, add it to the weapons collection.

Question:

When using an enum to filter out objects and place them in a Collection, as shown in the Clean Code example, is it a code smell?

I'm not checking for a specific attribute and then calling a method to perform an action, I'm simply filtering, not more, nothing less.

This might get a little messy, because every subclass of Character might have similar checks, but we can place all of those checks in a Rule object.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Andy Hunt's Rule #1: Always consider context.

The nice thing about programming is that there really is not ONE right way to do things. It all depends on the context. The solution I gave might work for now but it might become onerous to maintain given new kinds of requirements. Likewise, your solution might be perfectly fine as well. To me, it's not so much a matter of correctness as it is being easy to understand, test, and maintain. Yes, there is some level of aesthetics involved and what might look beautiful to me might not be so attractive to someone else. The point is, aesthetics should not be your main consideration; it should be readability, simplicity, testability, and all the other "-ities" that roll up under the big Q, Quality.

Take for example the snippet you cited from Clean Code. That came from the chapter about Naming. That is by far, in my opinion, the most important chapter of that book (and it wasn't even written by Uncle Bob. It was written by Tim Ottinger, if I'm not mistaken). The context for that code snippet was around renaming variables to make them more expressive. So, I'm not sure it's a good example to cite in this context.

However, like I said, it's possible that your approach would be decently clean. I'd just have to judge it in the context of the rest of the code that it would have to live with. I can tell you this though, your version could use some renaming as well. I would prefer to write it like this:


That code is more expressive. There's a smell there though in that the add() method also incorporates the concern of containing the rule for whether a weapon can be added to a character's arsenal or not. This feels like a violation GRASP (General Assignment of Responsibility Principles) There should be a clear separation of concerns in your program code and in this case, there's not.  If you refactor and delegate the responsibility of rule determination/adjudication to a different object, that might mitigate the smell.

Again, you have to consider everything in totality, not just little parts in isolation. Take a holistic view of your design so you can keep everything clean and cohesive.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
As a matter of fact, I'd refactor that even further to get rid of the negation in the condition:

Very subtle difference but I think it improves cognition significantly. I always try to avoid negation in boolean conditions and definitely double negatives.

How does renaming containsAttribute() to is() improve cognition? Well, it abstracts away the implementation detail that you leaked (that there's a collection of attributes somewhere that may or may not contain a matching value) and switches the reader's focus to the intent or meaning of having a matching value in the attributes collection.

What that all is really saying is that "A Weapon IS a long blade [if you go into its attributes collection and find a value that matches the LONG_BLADE enum]" The first bolded part is the intent or meaning. The second italicized part is the implementation. Try to give names that are meaningful not "implementation-ful".
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
As I see it, the crux of this whole discussion is whether or not you should check types explicitly. Using instanceof programmatically checks an object's type. Checking for inclusion of an enum in a collection where the enum values represent types or attributes is also a way of programmatically checking what kind of thing something is. This case may be in a little bit of a gray area, depending on what exactly the enum values represent. Some (many?) would argue that this is not a good approach and that you should prefer to use the language's type system and polymorphism mechanisms instead of conditional statements.

You might say it's a matter of principle and to large extent you'd be right. Do you want to write something that's expedient for you now, regardless of the design principles you might be violating? Or would you rather adhere to principles and maybe take the initial hit in being able to come up with a solution that works because coming up with a clean design is much tougher than just whipping out something that's not so clean but is workable?

It's all up to you to choose which path you take. I tend to favor sticking to principles and finding a way to get as clean a design as I can from doing so. It's a narrow, more difficult path to stay on but I think doing so is worth the rewards you get down the line.
 
Sveta Rosemond
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:As I see it, the crux of this whole discussion is whether or not you should check types explicitly or not. Using instanceof programmatically checks an object's type. Checking for inclusion of an enum in a collection where the enum values represent types or attributes is also a way of programmatically checking what kind of thing something is. This case may be in a little bit of a gray area, depending on what exactly the enum values represent. Some (many?) would argue that this is not a good approach and that you should prefer to use the language's type system and polymorphism mechanisms instead of conditional statements.



The way I see it is, you're checking the attribute of an object to determine how to proceed. In the Clean Code sample, you're correct, it's about naming, but the author included it in his book, so I doubt it isn't considered clean or bad code even if the naming of the variables and methods are correct. Also, how else would you gather a List of flagged cells if you can check it's state in someway?  

My point is, in both examples, we're asking the object, do you contain this attribute? Do you contain a boolean variable that's true? If so, here is what to do.  Consider this other example:

I might have a Wizard subclass that contains Mana user as a character attribute. I can also have a Farmer subclass and he/she is also a Mana user, but cannot be classified as a Wizard. They might cast spells, for example, to make their garden grow, but they aren't Wizards in the game world, they will not accompany you on a quest, attack with fireballs or heal you. Now I agree that you should have a Rule class, but how that Rule class is implemented is what's interesting. Instanceof might tell me I have a Farmer subclass, but checking the attributes might tell me a lot more. Not every Farmer might be a Mana user, so implementing an interface might not be a good idea, it might also violate LSP.  
 
Sveta Rosemond
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

The way I see it is, you're checking the attribute of an object to determine how to proceed. In the Clean Code sample, you're correct, it's about naming, but the author included it in his book, so I doubt it isn't considered clean or bad code even if the naming of the variables and methods are correct. Also, how else would you gather a List of flagged cells if you can't check it's state in someway?  



Sorry, typo!
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Sveta Rosemond wrote:My point is, in both examples, we're asking the object, do you contain this attribute? Do you contain a boolean variable that's true?


That's one perspective. There could be different perspectives or approaches to the same problem.

That's what the Visitor double dispatch pattern does. Instead of you asking the object, "Hey, are you a Wizard?" or "Are you a Warrior?" you're telling the language type system, "Hey, Java, you know what these things are, you direct that object to the right method that knows how to deal with whatever type of object it actually is."

Same problem, very different solution.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
There's actually an exercise when we train people on pair programming (my regular paying job is as an Agile technical coach -- I just hang out here for fun ) where we challenge them to find a solution to a problem where they only use OO and specifically, polymorphism. No if-statements allowed.

Maybe you could try doing that as an exercise and see what kind of different approaches you can come up with to solve the same problem. Try it on Conway's Game of Life, for example.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Related to "we're asking the object, do you contain this attribute?" there's also the Tell, Don't Ask Principle which says that rather than asking an object for information and then acting on that information (which in many cases breaks or violates the object's encapsulation), you should tell the object to do something, giving it any kind of information it needs to complete the task. That's a 180-degree turnaround in perspective.  

By telling the object something like "Hey, you there Character, whatever kind of character you are, here's some kind of Weapon. See if you're allowed to take it. Here's a Rule object that will help you figure that out" you relieve yourself, the programmer, of having to figure all that stuff out yourself. If you have a bunch of Characters, that translates to something like this:

No if-statements there. I'm dealing with objects and I'm telling them what they need to do. I give them what they need and they get the work done for me.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I bet I could easily add that kind of code into my implementation of the Visitor pattern with double dispatch, where I have a random set of Weapon objects and a random set of Character objects, then tell the Character objects to pickup the first Weapon they are allowed to carry. And I can do that without any if-statements that check for object type.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
For me, setting up more complicated machinations is fine if it allows you to hide complexity at a high level.  I don't want to deal with nitty-gritty details in high-level code. I want to have abstractions in place that allow me to deal with what needs to happen, not how it happens. If I can do that in my high level code, I'll work really hard to implement something that may be complicated but clean under the covers. I don't mind the extra work.

It's like the mechanic who fine tunes a car's engine, putting everything he can in there to make the driver's life a lot easier, except the mechanic is also the driver. When I'm tinkering around under the hood, I don't mind dealing with lots of gears and gadgets. But when I'm driving the car and trying to win the race, I just want to be able to push the pedal to the metal and go. Shift, steer, and maybe brake. That's it.  I hope that analogy makes sense.
 
Sveta Rosemond
Greenhorn
Posts: 7
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Related to "we're asking the object, do you contain this attribute?" there's also the Tell, Don't Ask Principle which says that rather than asking an object for information and then acting on that information (which in many cases breaks or violates the object's encapsulation), you should tell the object to do something, giving it any kind of information it needs to complete the task. That's a 180-degree turnaround in perspective.  



In the Clean Code example, he asks for information, if the flag is set to true, and acts on it by added it to a list.

The Wizard and Sword example, if an attribute is present then I act on it by either rejecting it or accepting it, if I were to place that logic inside the class, as you pointed out, I might be violating GRASP. so I Rule object is implemented.

In your opinion is encapsulation broken in either example?  
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Principles are a lot like proverbs: They're very true in some contexts but in other contexts they may not be the best thing to heed. Same thing with patterns. A good pattern in one context becomes an anti-pattern in a different context.

"Look before you leap" vs. "He who hesitates is lost"
"The early bird gets the worm" vs. "Haste makes waste."
"All good things come to he who waits." vs. "A stitch in time saves nine."
"Clothes make the man." vs. "Don't judge a book by its cover."

You have to use your judgement to figure out what's the best thing to do in your situation. Sometimes making that determination takes some experimentation.

Now, do either one of those examples you gave break encapsulation? If you take it in the strictest sense, I would probably say yes, they do. Does it really matter? It depends. Again, you have to take a holistic view. There's really no right or wrong; it just is or isn't better than other alternatives, given the current context.

I know that seems like a cop out but that's the best I can do for you right now. Maybe later (or sooner) I'll find a way to explain it better. For now, I'd say don't worry too much about what other people say is good or bad. Consider your alternatives and the arguments for or against. Then make a choice based on your own judgement. You're the one who has to live with that code. Make your bed and sleep in it.

I don't strictly abide by all the OO principles. I use them as guidelines and apply them when the situation seems to call for it. I'd be fine with the kind of code you cited from Clean Code in most situations. Again, to me it's about being able to understand the code, test it, and maintain it. Anything I can do to keep code as high level and abstract as possible without creating too much complexity is good. As much as possible, hide details down under the covers where they stay out of sight for the most part.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Waxing more philosophical, to me it's kind of like practicing martial arts, which I do also. I practice Aikido, in which the ideal is to have ai-nuke (mutual preservation) instead of ai-uchi (mutual destruction). Ai-uchi was the ideal for many Samurai, where they were willing to sacrifice life and limb as long as they killed the opponent. In ai-nuke, you control the situation and resolve conflict so that both parties are able to disengage unharmed.

If two martial arts masters meet on a narrow path and neither one of them wants to give way to the other, how does this story end? It all depends on whether they choose ai-uchi or ai-nuke.

I guess my point is that as a programmer, you need to develop a sensitivity to your code and design. If you listen to it carefully and use principles to guide you, you'll start being able to sense which way your code and design wants to go. In order to do this, you have to work very carefully and intimately with your code. I use tests to "feel" code and designs out. They become my experiments, telling me what works and what doesn't. Based on what I learn through the tests about my design choices, I make adjustments. I refactor so that the code makes more sense, so that I can relate to it when I read it and say "Yeah, that's what needs to happen" or "Yeah, that's how this behavior should be expressed." That takes a lot of time and practice, just as it takes years for practitioners of Aikido and other similar martial arts to develop the kind of sensitivity that allows them to blend with attackers and effortlessly direct their energy so that no harm comes to either attacker or defender.

Sorry if this all seems like nonsense but I just wanted to share where I'm coming from on these things.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Maybe later (or sooner) I'll find a way to explain it better.


I guess it's sooner then...

Regarding that method you cited from Clean Code, again, it all depends on the context. Let's look at it again:

By Andy Hunt's Rule #1, we have to consider context here. What class does this method belong to? What assumptions can we reasonably draw from this snippet?

1. The gameboard object is some kind of Iterable instance or class variable of the current class.
2. Perhaps this class is a Game class of some sort. Edit: Actually, I think it was supposed to be a MineSweeper class.
3. The Game MineSweeper class has a logical and natural affinity to the Cell class.
4. The Cell.isFlagged() method doesn't really give away any details of the implementation. "Is flagged" is still a high-level abstract concept. We can guess what the implementation is but that's not really useful nor necessary. We just know that a Cell can either be flagged or not.

Given the above, there's not really much to be concerned about with this class possibly breaking encapsulation of the related Cell class.

Now, it might be useful to try to approach this from an "Tell, Don't Ask" perspective and experiment with a different way to get this job done. But what is the job that needs to get done? Why does anyone outside the (presumably) MineSweeper class want to get a list of flagged cells. What do they want to do with it? Depending on the answer to this question, it just might be a code smell where you're breaking the encapsulation of the MineSweeper class. But we don't know because we have no idea what the surrounding code looks like. So we can only guess, which is not a very useful exercise nor does it move this discussion forward to a logical and reasonable conclusion.

So we're back to "It depends."

If we just want to guess for the sake of discussion, let's assume that whatever wants to do something with the flagged cells tries to do something that the MineSweeper class really should be told to do (Tell, Don't Ask!), like make the flagged cells reveal themselves. What can we do with this kind of code? It's definitely going to smell of Feature Envy. If that's the case, then we might refactor it to this:



There, we fixed the code smell. Now, whatever was previously asking the MineSweeper class for a list of flagged cells and then going through that list and revealing each one of them can now simply tell the MineSweeper object to do what it wanted it to do: reveal all its flagged cells. That's done by calling the new MineSweeper#revealAllFlagged() method. To me, that's more OO and it makes more sense.
 
Bartender
Posts: 1251
87
Hibernate jQuery Spring MySQL Database Tomcat Server Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Congratulations Sveta Rosemond,

Your question has made it to our Journal  

Have a Cow!
reply
    Bookmark Topic Watch Topic
  • New Topic