6

There's a lot of questions already about coding a badge system similar to SO, my question is different. Assume I have a web page system, badges / achievements, stored in the DB as a row with the achievement key (id), user id, and any other data.

My simple question is, where should I store the badge ID? I have one class per achievement with all the data and methods for testing if it has been earned. I imagine I may have dozens or hundreds at some point. I want the IDs to be used hard coded only once, and in one concise place, so I have no chance of accidentally changing them or mixing them up.

I could hard code them in the class, like

public int Key { get { return 15; } } // I'm calling it Key, not ID

but if I split my achievements among multiple files I don't want to have to run around looking for the highest Key when I add a new one and risk a mistake.

I could put them in some dictionary in another class...

public class AchievementSet
{
    private Dictionary<int, Achievement> _achievements;

    public AchievementSet()
    {
        _achievements = new Dictionary<int, Achievement>()
        {
            { 1, new SomethingAchievement() }
        };
    }
}

But now the class itself doesn't know its own key, and it needs to (or does it?) If I passed it into the constructor now I risk mismatching the numbers.

Any recommendations?

Tesserex
  • 17,166
  • 5
  • 66
  • 106
  • 1
    Why wouldn't you store the badges in the database? – Colin Pear Dec 18 '12 at 05:41
  • 1
    I suppose, because there's code associated to each badge and you would still need to match that piece of code with the badge ID. So it basically wouldn't solve anything. – guillaume31 Dec 18 '12 at 15:34

2 Answers2

2

In the context of Stack Overflow, I'd imagine each badge has properties such as: Id, Name, Class (Bronze, Silver or Gold) and Description etc.

You mention that you currently have a class for each badge/achievement, each with appropriate checks for conditions on which it would be awarded.

The reason I'm suggesting you move away from the model you're looking at now (one class per achievement) is because you're going to continue to face huge problems down the road when you're navigating through 200 different classes looking for that one ID you can't recall.

By storing your badges in the table, your data is all in one logical place and not scattered across your application.

In answer to the question: So do you disagree with the accepted answer to: stackoverflow.com/questions/3162446/

Not necessarily, and I like this idea more than my earlier proposal for a single class that would check all the badges based on their ID.

Despite its name, I believe RexM is not defining the CommenterBadge itself in that file and should have named it CommenterBadgeJob. (You'll notice it has none of the traits I've defined in my answer and inherits from BadgeJob). The obvious question is "How does each badge job know which BadgeId it corresponds to?"

I would have an additional unique field in my Badge called BadgeJob by which you could lookup a badge.

enum BadgeClass {Bronze, Silver, Gold}

//This class would be inherited from the database.
public class Badge
{
    public int Key {get;set;}
    public string Name {get;set;}
    public BadgeClass Class {get;set;}
    public string BadgeJob {get;set;}
    public string Description {get;set}
}

I would modify his code as follows:

public class CommenterBadgeJob : BadgeJob
{
    public Badge commenter_badge {get;set;}
    public CommenterBadgeJob() : base() 
    {
        //Lookup badge
        string badge_job_name = this.GetType().Name;
        commenter_badge  = db.Badges.Where(n=>n.BadgeJob == badge_job_name).Single();
    }

    protected override void AwardBadges()
    {
        //select all users who have more than x comments 
        //and dont have the commenter badge
        //add badges
    }

    //run every 10 minutes
    protected override TimeSpan Interval
    {
        get { return new TimeSpan(0,10,0); }
    }
}
RedBlueThing
  • 42,006
  • 17
  • 96
  • 122
JoshVarty
  • 9,066
  • 4
  • 52
  • 80
  • So do you disagree with the accepted answer to http://stackoverflow.com/questions/3162446/how-to-implement-badges?rq=1 ? – Tesserex Dec 18 '12 at 05:48
  • I've updated my answer. If I was unclear, let me know and I'll try and revise my answer some more. – JoshVarty Dec 18 '12 at 06:11
  • I've gone with putting all the data in a DB, which works well since the majority of my achievements will have simple conditions, like some field >= threshold. Thanks! – Tesserex Dec 20 '12 at 02:31
2

How about using an enum ?

  public enum BadgeTypes
  {
      GoodAnswer    = 1,
      Commenter     = 2,
      Teacher       = 3,
      //...
  }

Each BadgeJob could have a BadgeType property which would be used to populate the badge id when inserting an achievement during AwardBadges() (enum values can be persisted to integers).

I see no necessity for having one class per achievement. BadgeJob's contain all badge attribution logic and BadgeTypes suffice to represent the different badges.

guillaume31
  • 13,738
  • 1
  • 32
  • 51
  • But then isn't there one `Job` class per achievement? That seems like the same thing... I don't want one "Logic" class that's thousands of lines long. – Tesserex Dec 18 '12 at 13:42
  • Yes there is one Job class per achievement ("BadgeJob **'s** contain..." was plural). Sorry if that wasn't clear. – guillaume31 Dec 18 '12 at 14:30
  • Or, do you mean, you don't want one Job class per achievement ? I fail to see how this would be shorter :/ – guillaume31 Dec 18 '12 at 14:50
  • I do want one each because I don't want a single enormous class. But if the logic is in separate class, their data (name, description) can go in there too. – Tesserex Dec 18 '12 at 16:35
  • Name is contained in the Enum. Description can be a resource in a resx file and/or could be associated with the enum value (see http://blog.spontaneouspublicity.com/associating-strings-with-enums-in-c). – guillaume31 Dec 18 '12 at 16:50
  • 1
    Enum values can't contain spaces or punctuation though. But I see where you're going with it. Nice link too. – Tesserex Dec 18 '12 at 16:58