Partial classes vs other methods

[Cloud9_]

In my RPG, my Character class is getting WAY too big. I need to split it up, and I know of 2 methods for doing so... but which would be the best way? (btw this is using C#)

I could use partial classes;
public partial class Character, CharacterBattle, CharacterInventory, etc etc etc

I could also make a CharacterBattle, CharacterInventory, etc etc into their own objects and create them in the Character class. Which would you do and why?

[x1hundredregrets]

Partial classes are definately more elegant, after all, thats what they're there for. But essentially, both of those methods are doing pretty much the same thing. I would say which you think would be easier and/or neater.

[Cloud9_]

I'm actually thinking of doing it with objects, few people said it would be much easier to refactor later on and if i ever ran into trouble with the partial class it could be a nightmare tracking down bugs

[Faille]

I'm no programmer but seems to me that using classes may be the better option but one more beneficial when working in a team environment where you have different people working on different parts. If you're doing everything yourself then maybe it's not such an issue?

ps either way, comment / document everything! 8)

[Cloud9_]

well I didn't know about partial classes, so I thought about doing it that way to learn about those... but... thye're SOO easy lol, and I totally saw a lot of potential for headaches down the road. (although setting them up initially would have been SO much easier)

I'm just about done refactoring everything to work with the new system I'm setting up, only took a few hours, and at the same time I'm also refactoring my battle system to be cleaner so its good that I did this i guess.

Oh man, you should see my code comments... When I'm done working on something for the day i take some time to go through EVERYTHING and comment it all... A friend saw my code and was baffled "Why do you comment everything like this? the method name is pretty descriptive...!"

[Cloud9_]

actually, after playing around with it before owrk and coming home after work and playing around with it, i did the following;

Battle methods I put into a partial class (because I have a LOT of battle methods and I didn't want them in the main class, but with the way I have everything else set up, i didn't want to have to keep passing things like the 'animation class to it and what not. Was much easier to just make that a partial class.)

Inventory, Stats, and Magic I made objects used by the main class.

Figure this was the easiest solution for my problems

[slitz]

While I'm mainly a Java developer, and we don't have partial classes, after reading the description on them it seems like a pretty darn bad idea to me, so I would stay away from them as much as possible if I were you 8)

Seems to me (without trying to insult you here) that you somewhat lack the knowledge of object oriented design.
It doesn't make sense to me to have classes called CharacterBattle, CharacterInventory, unless they are classes that should be different from classes named Battle and Inventory (though Battle is really a bad name for a class as well, BattleManager comes to mind for instance) and they are subclassing Inventory and Battle.

A CharacterInventory class should instead be replaced by a function/method
in your character class code.

private Inventory inventory = new Inventory(); // either subclass a collection [bad idea imo] or use Composition [the better choice]

public Inventory getInventory(){
      return inventory;
}

public boolean addToInventory(Item item){
      return inventory.add(item);
}

Meaning, you should have a Inventory class not a CharacterInventory, but your Character class should obviously have a easily obtainable Inventory.
This might seem like nitpicking but you will get a lot more clean design and a design that makes sense in a OOP kind of view, which is better in the long run.

Battle methods I put into a partial class (because I have a LOT of battle methods and I didn't want them in the main class

Your main class is mainly only for starting up your main thread, there shouldn't be any logics in it that can easily be put in a class.

Inventory, Stats, and Magic I made objects used by the main class.

Jupp that's exactly what I'm talking about.

A note about a Battle logics class...
Obviously a class like this should be a singleton, without a bunch of static methods.

public class BattleManager {
 
    private static BattleManager instance = new BattleManager(); // to avoid multiple instances of it. If you instanciate it with getSingleton multiple threads *might* create multiple instances
  
    private BattleManager(){}

    public static BattleManager getSingleton(){
           return instance;
    }

   // your battlemethods! none static since the instance can be reached everywhere

}

Obviously, a class like this should still be used wisely and shouldn't be referenced in classes where it doesn't belong, ie Inventory for instance

[Cloud9_]

Seems to me (without trying to insult you here) that you somewhat lack the knowledge of object oriented design.

I've only been programming for a total of about 4 months, and while I understand the concepts of OO design I'm working on the practice of utilizing them. I pick things up extremely quickly, and while just about everyone I've met said "stop with game development, learn more about C# and OO stuff first" they were amazed at what I've been able to do in the short amount of time I've been programming. 4 months yes, but the dedication to make this an eventual career? absolutely. I've dove into this full force

It doesn't make sense to me to have classes called CharacterBattle, CharacterInventory, unless they are classes that should be different from classes named Battle and Inventory

actually I could just as well list these classes as "Inventory" or "inventoryManager" because thats all it does.. I called it "CharacterInventory" because when i was playing with partial classes I wanted the 'character' in there.. when I realized it was gonna be a headache, I switched it to objects and classes but never alter'd the name.

A CharacterInventory class should instead be replaced by a function/method
in your character class code.

Your inventory code is similar to what I have in my code.

        
        List(Item) inventoryItems = new List(Item)();
        List(Weapon) inventoryWeapons = new List(Weapon)();

        List(Armor) inventoryArmor = new List(Armor));
        List(Armor) inventoryShields = new List(Armor)();
        List(Armor) inventoryHelms = new List(Armor)();

        Weapon equippedWeapon = new Weapon();
        Armor equippedHelm = new Armor();
        Armor equippedArmor = new Armor();
        Armor equippedShield = new Armor();

        public Weapon EquippedWeapon
        {
            get { return equippedWeapon; }
        }

        public Armor EquippedArmor
        {
            get { return equippedArmor; }
        }

        public Armor EquippedHelm
        {
            get { return equippedHelm; }
        }

        public Armor EquippedShield
        {
            get { return equippedShield; }
        }

NOTE: Above, the arrows don't draw in the post, so I replaced them with ( )'s

I also have a few methods to add item to inventory, sell item, destroy item, equipp item, etc etc... I am ALWAYS looking for feedback on how to do things though, i dont want to get trapped into one mindset, i'd much rather have the thought process of several solutions so I can pick the one that works best for my situation. (one of the reasons I started this thread )

A note about a Battle logics class...
Obviously a class like this should be a singleton, without a bunch of static methods. // EDIT: code snipped out for space

I'm going to look into this, sounds like a great method for doing things. Thanks for the idea

Thanks for the help!

[The Ancient]

You can definately learn them concurrently, but yes, start utilizing OO as soon as possible. It's so much cleaner I can't even imagine not using it especially in the example you provided which seems so well suited towards it.

[Cloud9_]

Originally Posted by The Ancient

You can definately learn them concurrently, but yes, start utilizing OO as soon as possible. It's so much cleaner I can't even imagine not using it especially in the example you provided which seems so well suited towards it.

Well, when i'm done with the section I'm working on (battle screen) i'll post my code for you guys to nit-pick... I'd really be interested to see some thoughts on how to improve

EDIT: Also, I've tried to utilize OO design from day 1, my first thought was "if i will somehow be able to use this more than once in my code, put it into another method / class" and that snowballed into what i've been doing after reading the books I've read.

An example is my items / inventory... each item is inherited from a BaseObject class, so are weapons, armor, helms, spells, etc. Each Spell creates a new spell and is added to a list of "knownSpells", calling the function GetKnownSpells returns the list, and then it can be used to draw that list on screen in my draw method.

[Kharza-kzad]

I'd say if you've been coding 4 months and you are already trying for good oo design you are ahead of the game. I wrote horrible horrible code for at least 2 years after I turned pro, not to mention the 5ish years before that

Getting into the OO frame of mind was really hard for me, as I was an assembly / performance guy.

If you haven't, have a look at design patterns. They are useful, but you shouldn't try to shoehorn everything into a pattern. It's cool to see what the supposed "best attack" is for alot of common tasks though.

[Vinen]

If you really want to dig into Object Oriented design buy the Design Pattern book recommended in the C# XNA thead. Head First Design Patterns- it's good and incredibly helpful. I just finished running through the entire book. Still been too lazy to restart my project thought :|

[Cloud9_]

Originally Posted by Vinen

If you really want to dig into Object Oriented design buy the Design Pattern book recommended in the C# XNA thead. Head First Design Patterns- it's good and incredibly helpful. I just finished running through the entire book. Still been too lazy to restart my project thought :|

heh the way i figure it, im gonna finish my current project, read some more, and put those concepts to use in my NEXT project.

[slitz]

Originally Posted by Cloud9_

I've only been programming for a total of about 4 months, and while I understand the concepts of OO design I'm working on the practice of utilizing them. I pick things up extremely quickly, and while just about everyone I've met said "stop with game development, learn more about C# and OO stuff first" they were amazed at what I've been able to do in the short amount of time I've been programming. 4 months yes, but the dedication to make this an eventual career? absolutely. I've dove into this full force

I don't doubt that, it's very hard to change a habit however 8) and unfortunately I'm talking from experience...

Your inventory code is similar to what I have in my code.

        
        List(Item) inventoryItems = new List(Item)();
        List(Weapon) inventoryWeapons = new List(Weapon)();

        List(Armor) inventoryArmor = new List(Armor));
        List(Armor) inventoryShields = new List(Armor)();
        List(Armor) inventoryHelms = new List(Armor)();

        Weapon equippedWeapon = new Weapon();
        Armor equippedHelm = new Armor();
        Armor equippedArmor = new Armor();
        Armor equippedShield = new Armor();

        public Weapon EquippedWeapon
        {
            get { return equippedWeapon; }
        }

        public Armor EquippedArmor
        {
            get { return equippedArmor; }
        }

        public Armor EquippedHelm
        {
            get { return equippedHelm; }
        }

        public Armor EquippedShield
        {
            get { return equippedShield; }
        }

NOTE: Above, the arrows don't draw in the post, so I replaced them with ( )'s

While the idea is still good, this is probably the reason why you have such big character class (though, there is really nothing bad with big classes, as long as you don't lose the overview and as long as it doesn't take care of logics that it shouldn't take care of).

You don't need five collections / containers whatever you want to call it, for five different items. You want one in fact.

Assume you have a interface called Item with methods/functions that works for all items. Weight could be such function since all items have a weight if weight is relevant for your game, value of the item could be another one etc...
A weapon isn't very specific either so that could be a interface as well with methods such as getDamageRange() and armor isn't specific either so that could be its own interface with getArmorClass() method etc.

So in the end you have (this is just a extremely simple example ):

public interface Item {
   // methods here
}

public interface Weapon implements Item {
  // methods from item and Weapon here
}
public class TwoHandWeapon implements Weapon {
  // 2HWeapon, weapon and item methods here 
}

Same should be done with armor...
So what's the big benefit from this more than giving all your Item, Weapon and Armor a blueprint (though this is reason enough..)?

List(Item) inventoryList = new List(Item)();

private void fillInventoryWithRandomJunk(){

    Item item1 = new TwoHandWeapon();
    Item item2 = new ChestArmor();
    Item item3 = new Junk(); // assuming it implements the Item interface

    inventoryList.add(item1);
    inventoryList.add(item2);
    inventoryList.add(item3); 
}

All classes are subclasses of Item and thus you can add them to a container as items, why be more specific than that unless you have a very good reason for it of course?

Being the big pain in the ass I am, I would also have to comment on your Equipped stuff. I guess the large Character class is also a lot due to "design flaw" (note here, I'm really not trying to tell you that you suck, imo you've done pretty darn good for 4 months of programming, I'm just trying to give you some advice).
While when you think of a Character in game that he have his own equipment, his stats, his appearance etc, the logics of the different attributes shouldn't be in the Character class, rather they should be available from the character class.

Bad example (in character class):

private Color hairColor = Color.BLACK;
private Beard characterBeard = new Beard();

public Color getHairColor(){
    return hairColor;
}

public Beard getBeard(){
    return characterBeard;
}

How I think it should be done:

private Appearance characterAppearance = new Appearance();
// obviously, the different set methods should be called when creating your character

public Color getHairColor(){
    return characterAppearance.getHairColor();
}

public Beard getBeard(){
    return characterAppearance.getBeard();
}

Meaning, imo, the character class shouldn't include the logics for equipped stuff, another class should take care of that. Whether you want to call getEquippedWeapon on your character or have a manager for all that kind of stuff, is of course a preference however.
You could also of course, replace your List with a Map and let the boolean value decide whether it's equipped or not. Though, I wouldn't do it that way for larger projects, but for something small (and while still learning) I wouldn't dismiss it completely.

Hope you don't take this the wrong way now! And good luck with your project.

[Cloud9_]

You don't need five collections / containers whatever you want to call it, for five different items. You want one in fact.

I selected the different list route because it was the first solution that came to mind, and it seemed to make sense. I didn't think of trying to put everything in one, I thought that when I had to list all the items it would just do a foreach (Weapon w in weapons) type of loop, and that would draw the inventory screen for weapons.. although now that I look at it, the method you suggested will work just as well and reduce some clutter.

Being the big pain in the ass I am, I would also have to comment on your Equipped stuff. I guess the large Character class is also a lot due to "design flaw" (note here, I'm really not trying to tell you that you suck, imo you've done pretty darn good for 4 months of programming, I'm just trying to give you some advice).

I have VERY thick skin, nothing you (or anyone) says is taken as an insult, its more taken as constructive criticism ... so speak away, i encourage it. How else am I gonna learn ?

While when you think of a Character in game that he have his own equipment, his stats, his appearance etc, the logics of the different attributes shouldn't be in the Character class, rather they should be available from the character class.

Someone else just brought that up to me, and yea I've been putting all their stats and what not in one Character.cs class. Known for next project for sure, but I want to try and make it work for this project (being as how the project design is almost complete, would hate to re-design everything now if I can work around all my previous bad design choices)

Hope you don't take this the wrong way now! And good luck with your project.

Never, i appreciate the time taken for the criticism and will keep an eye out for thing sI can can improve.

Also, i did say above i'd release the source when the project was complete, this is NOTHING compared to the crap I'll prolly take for my coding choices when its actually complete > haha.. but I welcome it because I really am looking to improve