When one object can be used multiple ways

Started by
15 comments, last by Kylotan 6 years, 11 months ago

[Interfaces describe how a thing is to work, everything must be completely interchangeable. It IS-A thing. In all ways it is interchangeable with any other thing. For example, a 13/16 spark plug must be interchangeable with all other 13/16 spark plugs, even though they may vary in their implementation details.

This is a summary of the L in SOLID, the 5 core rules of OO.
Upon first reading these don't really give any practical advise as to how to fix your code... But it's worth keeping them in the back of your mind at all times so that eventually with experience you'll come to grok them.

If I hide the drinking of potions behind one method, how would I determine what attribute the potion was meant for? Health or Mana. Let's keep it simple, and say Health and Mana are int variables. How would I write my function to determine which attribute to update based on Potion, and then update it?

There's lots of ways. Perhaps Potions don't need to act upon the Character interface at all; instead they could act upon an Attributecollection interface which the Character implements. This is the D in SOLID - remove dependencies between components by creating a common interface instead.
The potion could have a name (enum, int, string, etc) specifying which attribute it wants to act on. It could ask the AttributeCollection to retrieve an attribute with that name, or null if it doesn't exist,and then the potion can then apply a change to that attribute. This would also allow potions to be used on any other game object that implements the AttributeCollection interface and contains a "mana" attribute.

Shouldn't the character be aware of what weapon it's using? and who it's using it on?

In your example, this is true but it's also true that the code that is controlling the character is also aware of these details... Which is a hint that the character abstraction is pretty weak.
If the character is fully aware of its weapon and its enemy, then the controlling code could just write:
character.attack(); // it knows which weapon and who
Advertisement
There's lots of ways. Perhaps Potions don't need to act upon the Character interface at all; instead they could act upon an Attributecollection interface which the Character implements. This is the D in SOLID - remove dependencies between components by creating a common interface instead.
The potion could have a name (enum, int, string, etc) specifying which attribute it wants to act on. It could ask the AttributeCollection to retrieve an attribute with that name, or null if it doesn't exist,and then the potion can then apply a change to that attribute. This would also allow potions to be used on any other game object that implements the AttributeCollection interface and contains a "mana" attribute

Would something like this be adequate?


public class Potion {
    private final String potionName;
    private final int increaseBy;
    private final String potionType;
    
    public Potion(String name, int increase, String type){
        this.potionName = name;
        this.increaseBy = increase;
        this.potionType = type;
    }
    
    public int getincreaseBy(){
        return this.increaseBy;
    }
    
    public String getPotionType(){
        return this.potionType;
    }
   //Character class
   AbstractMap<String,Integer> characterAttribs = new HashMap<String,Integer>();
   characterAttribs.put("Health", 10);   
    public void Drink(Potion potion){
        if(potion.getPotionType() == "Health"){
            //grab the heath attribute by the key, increase it and put it back.
        }
    }

I've followed you're advice and rewrote my character interface as you described.


interface Character{
  void attack(Enemy enemyObj);
}

How then, would I allow my character to interact with the weapon so that it could reload if it wanted to? I understand the character knows the weapon, but it doesn't know the Weapon is reloadble. How would I let the Character know it's reloadable, and allow the character to do it?

Your Drink implementation has an unnecessary step - instead of checking each name explicitly, just look up the name provided by the potion in your characterAttribs - if it exists, you have the attribute you need to modify.

I covered the reloading aspect above, though perhaps not clearly enough.

If the Character wants to know if a weapon is reloadable, it simply asks it. e.g. `bool Weapon::IsReloadable()`. (I called it 'needsReloading' in an earlier post.) This returns true or false depending on the type of the weapon.

Your Drink implementation has an unnecessary step - instead of checking each name explicitly, just look up the name provided by the potion in your characterAttribs - if it exists, you have the attribute you need to modify.

Clarify this a little. How would I get the name without calling the getter?

I didn't say "without calling the getter", I said without "checking the name explicitly".


public void Drink(Potion potion){
    if (characterAttribs.containsKey(potion.getPotionType()) {
        characterAttribs[potion.getPotionType()] += WhateverAmount;
    }
}

I didn't say "without calling the getter", I said without "checking the name explicitly".


public void Drink(Potion potion){
    if (characterAttribs.containsKey(potion.getPotionType()) {
        characterAttribs[potion.getPotionType()] += WhateverAmount;
    }
}

I was just about to update my code the the same sample!

Thanks man! I mean it, it's been a rough few days attempting to figure all this out. At least on this forum, people are patient enough to help, and even provide sample code. Thanks for the links, and explanation. Here's a vote up :D

One more question, given that any Weapon that implements the Weapon interface has to implement all the methods, what should I do if a Weapon isn't reloadable, and therefore can't replenish, what should go in that method (Obviously, as you pointed out, it does nothing, but you didn't say "leave it empty", so do I throw an exception?

You have to follow the rules set by the interface. If the interface allows calling "reloadWeapon", even if "canReload" returns false, then yes, expect that at some point it may get called, so generally I'd simply do nothing, as that's as good as it gets for a non-reloadable weapon.

If the interface forbids calling "reloadWeapon", if "canReload" returns false, then you can basically do anything you like in "reloadWeapon", since if it gets called, you are in undefined behavior territory, so anything you do or don't do is ok. This seems like a good point to kill the user randomly, or win the lottery, crash, or something else.

In the more sane behaviors however, depending on how paranoid you are, you either implement it as empty method (you shouldn't c<all it, but no harm done), or you assert "You shouldn't get here!". That's useful for development, so you notice you're doing things you're not supposed to do. In a release, asserts should do nothing, so a user will not see your assert message if he/she trigger a bug that ends up calling that method.

I'd be tempted to put an assert in there, because calling it would be an error that you need to fix as a programmer, rather than an exceptional condition that you need to deal with and recover from at runtime. (If it does happen in a release build, then doing nothing at all is reasonable.)

This topic is closed to new replies.

Advertisement