1

I was reading here about OOP and methods, and the accepted answer states that method names should be verbs. However, that doesn't really answer my question.

Suppose if I had a Character class with a private List inventory.

public class Character {

     private List<GameItems> inventory;

     // constructor and other methods left out.

     public boolean checkInventoryfor(GameItem item)
}

Now, suppose at various points in my game, the player wants to check their inventory for a specific item, the checkInventoryfor(GameItem item) can be called.

I've been told that the name is a code smell, because it gives away the fact the class has a collection (inventory) that needs to be checked. A better name would be has(GameItem item). has() flows better in terms of language because most likely you'll have an if statement that reads if(character.has(sword)) {// rest of code here}.

Either way, you know that you're querying a collection, of what type? We don't know, we don't care. Neither method tells you, all it returns is a boolean.

Can method names leak implementation details and break encapsulation?

Improve this question

4 Answers 4

Reset to default
17

The actual inventory is still hidden from outside classes (i.e., the List<GameItem> inventory is private), so encapsulation is not broken. Whether it makes more sense to name the method has(GameItem item) or checkInventoryForItem(GameItem item) comes down to your requirements. If there is a difference between a character having an item in inventory vs having it in her hand, has(item) might be ambiguous, while if there is no notion of "equipping" items, has(item) might suffice.

Presumably somewhere else in the class you have a method to add an item (to the inventory), possibly also to equip an item or otherwise move it to/from the inventory. These all dance around the notion of the character having an inventory, regardless of what you name them. The fact that a character has an inventory is part of your domain model and is not a secret (most people would probably expect it given the domain), but the fact the inventory is represented by a List<GameItem> is an implementation detail that is hidden.

The key to encapsulation is to not give unfettered access to that inventory so that other objects can modify it at will. You provide specific methods to perform specific modifications or queries (like checking whether an item exists in that inventory, or adding/removing an item) on that inventory.

Improve this answer
2
  • +1. You might want to explicitly stress what you mentioned in passing, that while List<GameItem> is an implementation detail, inventory is part of the domain model. Commented Jan 31, 2019 at 14:41
  • You make a good point about having something in your hand, or wearing it on your person. "Having" an object doesn't necessarily mean it's in your backpack. So method behavior is still very much implementation-specific. Commented Jan 31, 2019 at 15:01
4

A method can certainly break encapsulation and leak implementation details. Taking the punching bag example from your linked answer, you should definitely prefer fill over fillWithSand, since the caller doesn't really care what the bag is being filled with, and different bags might be filled with different things (on the other hand, one bag could have multiple valid things it could be filled with that the caller has to choose, so YMMV). If your Inventory were instead a Map<GameItem, int>, you wouldn't want to call the method to retrieve an item hasGameItemKey, since that's an implementation detail that is both irrelevant to the caller and subject to change without changing the interface.

As a method name, has doesn't actually explain what it is checking whether the character has. It could be talking about a feat, a quest, a body part, etc. So, if you want to change the name, I'd suggest hasItem instead if the character could "have" anything that isn't an inventory item.

The criticism you've received seems like it may not apply well to your particular case, however. In the case of a game, the designers at all levels need to know how the game works - whether the character has an inventory, whether the inventory can contain the sword they are checking for, etc. Including the word "inventory" can also help differentiate between different possible places an item could be - for example, you might want to check whether a character has a particular item equipped, in their inventory, or either.

So, the answer, as always, is that you need to analyze your particular software and figure out what method name is the simplest you can make, but no simpler, is the most generic you can make, but no more generic, and most importantly, makes sense to the person calling it.

Improve this answer
0

By using the name checkInventoryFor(), you're establishing that a character has an inventory. In the application you're using, should a character have an inventory? Would the user think in terms of a character's inventory?

If you're writing a game, the developers and players would usually expect the player character to have an inventory, and if other characters have inventories that would be natural and easily understandable. In this case, checkInventoryFor() is a reasonable name. (You might want to have other ways of searching it, of course. If you need to know if the player has a light source, you want to have some method other than checking the inventory for a candle, then a lantern, then a flashlight....)

If you're writing software to help an author write fiction (such as Scrivener), then characters wouldn't normally be considered to have inventory, and something like has() would be preferable.

Improve this answer
0

Your particular example has already been discussed in other answers.

In Spring Data (a Java framework) there is an interesting concept for building working repositories:

You only need to write repository interfaces and follow conventions when naming methods. For example you may write an interface method with the signature Person findByName(String name) and if the Person entity has a property called name a working implementation of that method will be created (via proxying) at runtime for you.

In such use cases (dynamic proxies) method names may indeed affect implementation details. It's up to you to judge whether this also means leaking implementation details. Probably it's not breaking encapsulation.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.