Determine the Object Type with a Collection

Started by
11 comments, last by Stevepainter 5 years, 2 months ago

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?

For my question, lets remove the Rule object and stick with two objects, a Wizard and Sword. A Wizard is a subclass of the abstract Character class and Sword is a subclass of the abstract Weapon class. The Character class will have a method called tryAdd();
 


public final boolean tryAdd(Weapon weapon){
     // Code to add Weapon
}  

At first I thought I would use an enum to determine what object the player is attempting to add to it's inventory.

public enum WeaponType {
    ReloadableWeapon // -- imagine more weapon types
}
public abstract class Weapon 
{
    private WeaponType wt;

    public Weapon(WeaponType wt)
    {
       this.wt = wt;
    }
}

public ReloadableWeapon extends Weapon{

     public ReloadableWeapon()
          super(WeaponType.ReloadableWeapon);
     {

     }
}

As pointed out, it violates DRY and is a poor version of instanceof. Okay, fine, the comments suggested a Map or Collection of attributes, even this GameDev link suggested the same thing. 

So a long the lines of this:

public enum GameObjectAttributes{

    Replenishable
    Destructable
    Health
    LongBlade
    ShortBlade
    Mana
    Health
    Potion
    Indestructible
}
public abstract class Weapon 
{
    private List<GameObjectAttributes> gOAttributes;

    // imagine a constructor :D.

    public final boolean containsAttribute(GameObjectAttributes attribute)
    {
        // determine if weapon contains a specific attribute. 
    }
}

Suppose I had a Sword object, and the level of detail allows a Sword to be damaged and broken if hit hard enough, so my Sword object might contain Destructable and LongBlade as attributes.

As suggested in this link, this too is a bad idea because I'm using the attributes to determine or carry out behavior with that object.

HOWEVER!

Isn't this how OOP is supposed to work? I query an object for an answer, in this case true or false and carry out actions with that object? In this case I ask the Weapon, do you have LongBlade as an attribute? If so, the Wizard cannot add it to there inventory. The method doesn't add the weapon and returns false. 

Problem

Even if we were to use a Rule class, at some point, we have to determine what object is being passed to the TryAdd() method, in other words, one object will have to communicate with another object to determine the object type. The link that suggests the attribute collection is a bad idea, says that a Player should implement a tryCarry method, but even that methods needs to decided if the Weapon parameter is a Sword.

How then do you make that distinction without instanceof or a variable to decided what to do? Or How do two objects communicate with each other so that one object is aware of the other object type without violating OOP principals? Can someone provide a clean code sample?

This problem is getting frustrating to solve because no matter what solution I come up with, someone will point out it's a bad idea, but no one can provide a solid solution to the problem. Even Eric Lippert's blog just suggests a Rule class but no example code of how to implement it. 

What seems like a simple solution to a problem is ripped apart by veteran developers who are quick to point out something is a code smell or bad code. Sometimes I wonder why bother writing any code at all. 

Advertisement

Wow, what a forum. 303 views and no one kind enough to answer.

Thankfully, I'm abandoning this train wreck of a forum.

To all the developers who think they're going to make it in the gaming industry, you aren't. Abandon that dream right now. This forum and that industry are pointless.

To hell with it, I hope this forum crashes and burns into ashes never to be seen again on the internet.

 

Die slow and painful Game Dev Forum.

Hi. I personally don’t know the answer, perhaps the other people who saw your post so far also didn’t. Perhaps you’re not that stupid and it’s a difficult question.

tbh, I find it a hit harsh to burn down the forum because you feel your question’s not answered quick enough. Speaking from experience, the forum and a lot of people on it are pretty helpful.

Crealysm game & engine development: http://www.crealysm.com

Looking for a passionate, disciplined and structured producer? PM me

On 10/1/2018 at 7:59 PM, BasementJoe said:

Even Eric Lippert's blog just suggests a Rule class but no example code of how to implement it. 

Just for weapon or ever item (that is design specific to fast up a rule implementation ) make a id of its kind as parameter. For character make a array of ids of weapons(items) that it able to use. It array is rules. Than on tryAdd just search array for weapon(item) kind id and return is it found.

#define if(a) if((a) && rand()%100)

Doesn't C# have the "is" operator?  So if Sword extends from Weapon then:


if (weapon is Sword)

should suffice for determining weapon type shouldn't it?

Or maybe I'm just another person who has misunderstood?

Quote

What seems like a simple solution to a problem is ripped apart by veteran developers who are quick to point out something is a code smell or bad code. Sometimes I wonder why bother writing any code at all. 

Seriously?!  Are you doing this professionally or as a hobby?  If the latter, then why do you care so much about what others think of your code quality?  Does your solution work?  If yes, then who cares if it's 'bad code'.  Honestly, every developer under the sun has their own opinions of what is good code and what is bad code.  I've worked professional in an environment like that (too many egos and pedants) and it was probably one of the least productive places I've ever worked; fearful of touching the keyboard cos you know you'll be criticised and have your code ripped to shreds.  It's not healthy!

Quote

To all the developers who think they're going to make it in the gaming industry, you aren't. Abandon that dream right now.

No shit Sherlock.  I can't speak for others but I do it mainly because I find it fun.

2 hours ago, Greedy Goblin said:

Honestly, every developer under the sun has their own opinions of what is good code and what is bad code.

I'm beginning to realise just how big an issue this is.  

18 hours ago, Greedy Goblin said:

Doesn't C# have the "is" operator?  So if Sword extends from Weapon then:



if (weapon is Sword)

should suffice for determining weapon type shouldn't it?

The point is that is a code smell. You end up with basically a massive switch statement. 

 

18 hours ago, Greedy Goblin said:

If the latter, then why do you care so much about what others think of your code quality?

Because you want to get better? 

 

18 hours ago, Greedy Goblin said:

Does your solution work?  If yes, then who cares if it's 'bad code'. 

It depends on what you mean by "work". If the solution works and it's ugly, well, sometimes ugly code is a necessary evil. But if the code is (problematically) slow, error-prone, or hard to maintain, then yes, you should care if it's "bad code".

 

18 hours ago, Greedy Goblin said:

Honestly, every developer under the sun has their own opinions of what is good code and what is bad code.  I've worked professional in an environment like that (too many egos and pedants) and it was probably one of the least productive places I've ever worked; fearful of touching the keyboard cos you know you'll be criticised and have your code ripped to shreds.  It's not healthy!

There's a difference between people who are trying to help you by sharing their experience and those who are just trying to show how much better they are than you. Unfortunately, telling them apart is something that often only comes from experience. 

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight
On 10/2/2018 at 2:59 AM, BasementJoe said:

Isn't this how OOP is supposed to work?

OOP says to prefer composition instead of inheritance when possible. 

Is there actually a good reason to be using abstract base classes here? What algorithms are simplified by it? 

1 hour ago, ChaosEngine said:

The point is that is a code smell. You end up with basically a massive switch statement

A code smell is not necessarily a bad piece of code, just something that needs further investigation in its own context if, like you say, the code is not performant or becoming bloated or overly complicated. That does not mean "if... is..." is necessarily wrong. 

My point was not to beat yourself up so much over your code quality. It's not healthy and very rarely helpful in my experience. 

If you have lots of If statements but your code is performing to your requirements and easy to read/maintain (which is of course subjective) then I fail to see what the problem is. Guides to writing good clean code are exactly that, guides, not some religious mandate (which is how some people seem to treat them). 

Thanks goodness I don't work in an environment like that any more. ?

Just have two separate collections


std::set<Sword> swords;
std::set<Staff> staffs;

They can have a common base class if you like. However, both being long pointy things is not enough reason imho to keep them together.

This topic is closed to new replies.

Advertisement