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.
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?
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.
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:
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.
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.
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.