1

I asked a question about how to structure an interface given that an object can be used in more than one way. The answer given was fine, but it didn't really address my primary problem. I decided to try for myself, and then come back with a more clear example/question.

Take for example this Character interface:

public interface Character {
   void attack(Weapon toAttackwith, Character enemy);
   void reload(Reloadable reload, Replenishitem replenish);
}

public interface Weapon {
    void attack();
}

public interface Reloadable {
    void replenish();
}

and Implementation:

public final class ReloadableWeapon implements GameItem, Reloadable, Weapon {

    @Override
    public void attack() {}

    @Override
    public void replenish() {}
}

A game weapon can be used to attack an Enemy, however, not all Weapons are reloadable. So in my character interface and created a method called reload with certain parameters. Reload doesn't necessarily just mean weapons, potion bottles/containers can be reloaded (refilled, two different words for the same behavior).

Is adding the reload method in the character interface considered code smell?

Community
  • 1
  • 1
  • I think you need to ask yourself the question if "attack" and "reload" is a functional of `Character` or not. In your context it might be, but a `Character` is probably "equipped" with weapons (or items at the lowest level). The weapon can cause damage, but may be influenced by the `Character`s abilities, that "potential" damage is then applied to another `Character`, which may be reduced (or increased) by what the character has equipped (armor) or other abilities. To my mind, the "combat" is almost another class - but that's me ;) – MadProgrammer May 08 '17 at 23:35
  • @MadProgrammer - The character is doing those things with the items, the character is attacking a target with a weapon, a character can reload a weapon. If a character is in fact doing these, it would make sense to have the reload method in the character interface, correct? It's a behavior a character can do(enemy or otherwise). –  May 08 '17 at 23:42
  • `reload()` and `attack()` are actions of a `Character` so make sense to be in the `Character` interface. The behavior of the methods, for example `attack()` should depend on the currently equipped `Weapon`. Perhaps consider having abstract classes `ReloadableWeapon` and `NonReloadableWeapon` which define the reload action of all sub weapons, i.e. all `NonReloadableWeapon`s do nothing. Also consider researching the Decorator and Strategy design patterns. – d.j.brown May 08 '17 at 23:53
  • As I said, that's your context, if that's how you engine works, then that's fine. I guess I'd be concerned about the "reload" method in `Character`, as you stated, not all weapons are reloaded so in my mind, it doesn't quite make sense - to me, how you might get around it I'm not sure – MadProgrammer May 08 '17 at 23:55
  • @MadProgrammer - What bother's you about it? How would you write it? Remember reload can take containers as well, not just Guns. –  May 08 '17 at 23:59
  • @d.j.brown - I had it as an abstract class but the inheritance was getting deep. So weapon was changed to an interface –  May 09 '17 at 00:00
  • 1
    @Nexusfactor First, take it all with a pinch of salt ;) - I simply have a different way of thinking about how I might handle it, but the context is light on the ground, so I might try and do things differently from you or how your engine is implemented. I just look for separation of responsibilities wherever I can and I'd be worried about the concept of a `Character` having a "reload" function - maybe a more abstract action would satisfy me, where it might be something alone `perform(Action)` and "reload" would be a separate action. Is it a good idea in your case, no idea – MadProgrammer May 09 '17 at 00:07
  • If you have to ask "how can I use these interfaces better?" ... then it may be true that your project could benefit from more time spent developing better and more usable APIs. The mark of a good API is that it should define a useful type. If the type an interface defines isn't all that useful to you ... well .... – scottb May 09 '17 at 00:10
  • @MadProgrammer - If I wanted to do it your way, for the sake of learning perform would take action parameter, and the object on which to do the action, correct? Let's keep it simple, and make action a String, how you implement reload? Check the string and perform the action? –  May 09 '17 at 00:11
  • @MadProgrammer - My primary concern is handling object that could behave in more than one way. A game weapon could be used to attack and a character could reload. –  May 09 '17 at 00:14
  • 1
    *"My primary concern is handling object that could behave in more than one way."* ... it sounds to me like your use case should have a type hierarchy. Each level of the hierarchy describes an important new detail of the child type. Have a look at the type hierarchy for the `Label` class in the JavaFX framework to get a better idea how this looks (and how it might be useful to you). – scottb May 09 '17 at 00:17
  • 1
    @Nexusfactor An "action" would need to be more complex, for a reload action, you'd need more information, like the weapon and the availability of ammo for example. To my mind, the "reload" action is the responsibility of the weapon, but it needs to take "ammo" (for example), which needs to come from the `Character` in some way, how is a complex question, which is why I might consider a "action" based API, where you might have a "reload", "attack", "heal", etc actions which can be applied to a character to perform, but which would require an instance of `Character`, ie `action.perform(self)` – MadProgrammer May 09 '17 at 00:27
  • @d.j.brown - I'm looking at the strategy pattern. Maybe on to something here. https://dzone.com/articles/design-patterns-strategy –  May 09 '17 at 00:27
  • "First, take it all with a pinch of salt ;)" is exactly what I'm going to do :) For my simple game, it's enough to have reload in the character interface. –  May 09 '17 at 00:38

0 Answers0