0

Suppose I have game achievement class that stores achievement information like name, popup message, etc.

class GoldMedal implements GameAchievement {
  @Override
  String getName() { ... }

  @Override 
  String getAchievementDisplayMessage() { ... }

}

Then when the game ends, I want to record current achievements to local storage (SharedPreferences) via static keys like ACHIEVEMENT_GOLD_MEDAL:

editor.putBoolean(ACHIEVEMENT_GOLD_MEDAL, isAchievedGoldMedal);

Question: Where should these keys be stored at?

It's tempting to put it inside GoldMedal since it's directly relevant and intuitive, but feel like it violates SRP (or no?) since it's related to storing mechanism:

class GoldMedal implements GameAchievement {
   // ...

  @Override
  String getStorageKey() { ... }
}

// Usage:
editor.putBoolean(achievement.getStorageKey(), isAchieved(achievement));

Another way I thought of is to hide it under an interface:

interface LocallyStorable {
  String getStorageKey();
}

class GoldMedal implements GameAchievement, LocallyStorable {

  @Override
  String getStorageKey() { ... }
}

I guess my question in general is what's the best practice to separate responsibilities that seem to be related to a shared entity like GameAchievement?

For example, when there's a new property of a GameAchievement in another domain, say in a tutorial page (String achievementTutorialText), should we make another interface out of it?:

interface GameTutorialComponent {
  String getTutorialText();
}

2 Answers 2

1

The Real SRP

You asked about the "Single Responsiblity Principle", but I've often seen some questionable interpretations that just happen to fit what somebody wants to put in one class. However, the author defined a responsibility pretty well:

A class should have only one reason to change.

In that context, I see 2 responsibilities here:

  • Define what data an achievement (in your running application) consists of
    The one reason to change would be "the data that makes up an achievement changes". An example change would be adding an int for a score bonus or removing the display name.

  • Persist player data
    The one reason to change would be "how we persist player data changes". Examples for changes would be that you're going to store everything in an encrypted text file from now on, or on a server.

So, "Persist player data" is an additional responsibility that should go somewhere else.

Example

Here's a concrete idea how to remove the responsibility from your GoldMedal.

public class AchievementKey {
    // If you really only have 2 achievements, this class is overkill and the
    // constants can just be part of PlayerDataStorage.
    // However, it wouldn't be unexpected if a game has >20 achievements nowadays.
    // In that case, this "key registry" class really helps.
    public final static string GOLD_MEDAL = "medal-gold"; 
    public final static string SILVER_MEDAL = "medal-silver"; 
}


public class PlayerDataStorage { 
    public List<string> readAchievements() {...};
    public void setAsAchieved(string achievementKey) {...};
}

// Usage
public void UpdateAchievements(int score){
    if (score > 500) playerDataStorage.setAsAchieved(AchievementKey.SILVER_MEDAL); 
    if (score > 1000) playerDataStorage.setAsAchieved(AchievementKey.GOLD_MEDAL);
}

Now the information for how player progress is stored is not inside your GoldMedal class anymore. Now any changes you make to the GoldMedal or other achievement classes will not change anything about how the data is stored. That's the point of this example - your eventual solution will depend on how the rest of your app interacts with achievements.

1

In your case introducing a LocallyStorable interface seems to be a good way to proceed.

When your method will need methods related to GameAchievement they will work against GameAchievement interfaces. When it will be time to persist your state the method responsible for it will work against a LocallyStorable interface. Another advantage of this is if you ever need to persist something which is not a GameAchievement the LocallyStorable interface will already exist and be used to handle the persistence. So the only thing that you will need to do is implement this interface on your new class needing persistence.

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.