Class Design with Polymorphism and Inheritance

Question

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.

Naming a class "Class" is bound to create major confusion down the road. — anthony sottile
– anthony sottile, Commented Aug 7, 2011 at 17:40
Please don't prefix your titles with "C#". That's what we have tags for on Stack Overflow. — John Saunders
– John Saunders, Commented Aug 7, 2011 at 17:49

Anders Abel · Accepted Answer · 2011-08-07 18:05:20Z

2

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;
}

edited Aug 7, 2011 at 18:05

answered Aug 7, 2011 at 17:42

Anders Abel

69.6k18 gold badges154 silver badges221 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

zeboidlund Over a year ago

Could you give me an example?

frenchie · Accepted Answer · 2011-08-07 18:12:00Z

1

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

edited Aug 7, 2011 at 18:12

answered Aug 7, 2011 at 17:59

frenchie

52.3k117 gold badges321 silver badges532 bronze badges

1 Comment

frenchie Over a year ago

Ok, cool. It's all about how many times you're going to be rewriting your code:) Happy coding!

Frederik Gheysels · Accepted Answer · 2011-08-07 17:37:16Z

0

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.

answered Aug 7, 2011 at 17:37

Frederik Gheysels

57.2k12 gold badges103 silver badges156 bronze badges

Comments

Community · Accepted Answer · 2017-05-23 12:19:51Z

0

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.

edited May 23, 2017 at 12:19

CommunityBot

11 silver badge

answered Aug 7, 2011 at 18:20

Mark H

13.9k4 gold badges33 silver badges45 bronze badges

Collectives™ on Stack Overflow

Class Design with Polymorphism and Inheritance

4 Answers 4

1 Comment

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related