CODEWITHSUNDEEP
c#inheritancepolymorphism
zeboidlund
zeboidlund

Reputation: 10137

Class Design with Polymorphism and Inheritance

I currently have the following:

public abstract class CharacterClass
    {

        public abstract Attribute FirstAttributeBonus { get; }
        public abstract Attribute SecondAttributeBonus { get; }

        protected Attribute[] attributeBonuses; //2 attribute bonuses, which add 10 to the attributes stored in the array.
        protected SKill[] majorSkills; //Major skills for class begin at 30.
        protected Skill[] minorSkills; //Minor skills for class begin at 15.

        protected IDictionary<int, Character> characterList; //List of characters who apply to this class specifically.

        public CharacterClass()
        {

        }

    }

With the idea in mind that whichever class I create will inherit from this base, and also inherit the classes fields as well. For example, one class could be Warrior; the other could be Battlemage, etc.

Is this the right way to perform such a design, while having the derivative constructors initialize the fields? Or is it better to write the classes out without them inheriting these fields?

Edit:

I forgot to mention that all derivatives will be singletons, and I'm changing the name of Class to "CharacterClass", to avoid confusion.

Upvotes: 0

Views: 243

Answers (4)

Mark H
Mark H

Reputation: 13897

I don't think there's anything particularly wrong with holding protected abstract fields if they are common to all derived classes, though I do recommend the solution that Anders has given.

The part I particularly dislike though, is the IDictionary<int, Character> characterList;.

I'm going to make the assumption that your Character has internally a CharacterClass reference so that you can access the details whenever you need, and as it appears, you will be creating a cycle between Character and CharacterClass. It seems like you're making the CharacterClass have too many responsibilities. I would move the responsibility of holding all Characters of a specific CharacterClass elsewhere.

Upvotes: 0

frenchie
frenchie

Reputation: 51927

You absolutely need to get a hold of the book called Head Start Design Pattern. You don't need to read the entire book: its first chapter describes the design pattern you are looking to implement.

Basically, you want your Character class to have interfaces that call classes that hold your implementation code such as the skill classes, attribute classes and all the other ones you'll add later. That way, you can add new types of skills and attributes and you'll also be able to modify those at runtime. You DON'T want the character class to hold all the possible implementations. For what you're trying to do, you want to favor object composition instead of inheritance.

Take 20 minutes to read the first chapter: http://oreilly.com/catalog/9780596007126/preview

Upvotes: 1

Anders Abel
Anders Abel

Reputation: 69260

I assume that you don't mean whichever class but rather all game object classes? In that case it might be a good design, if all game object really need those attributes.

The role of an abstract base class is to gather common code there to get rid of repetitions in the subclasses. If all game object subclasses need those fields then it is correct to put them in the base class. If the different game object subclasses initialize those to different values it is correct to defer initialization to the subclasses.

One possibility to force initialization of those fields is to provide a nondefault constructor in the base class, requiring the subclasses to pass init values as parameters to the ctor.

I assume you mean a constructor example? Change your existing

public CharacterClass()
{
}

into something like

public CharacterClass(Attribute[] attributeBonuses, 
  SKill[] majorSkills, Skill[] minorSkills)
{
  if(attributeBonuses == null || majorSkills == null || minorSkills == null)
    throw new ArgumentException("Null values are not allowed");

  this.attributeBonuses = attributeBonuses;
  this.majorSkills = majorSkills;
  this.minorSkills = minorSkills;
}

Upvotes: 2

Frederik Gheysels
Frederik Gheysels

Reputation: 56934

I think that this is smelling. This code-smell even has a name: God - class.

IMO, it is not a good idea to create one 'mother' (or god) class, where all other classes inherit from.

I see in your base class some properties like MajorSkills. I see that you'll have a class 'BattleImage' that you'll inherit from this base class, but, I don't think that an image has skills. I would create more specific base-classes, and only inherit from these base-classes if there exists an Is A relationship.

Upvotes: 0

Related Questions