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.
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:
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?
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?
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?
salvin francis wrote:
Feel free to criticize
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?
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.
salvin francis wrote:Just wanted to share what popped into my mind :
Feel free to criticize
salvin francis wrote:I just read that OP mention the DRY principle... My code looks horrible now
Sveta Rosemond wrote:To my anyways, it seems like a lot of code for something that seems to simple.
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't check it's state in someway?
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?
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.
Maybe later (or sooner) I'll find a way to explain it better.