Is My Class To Bloated

[ExponentialP]

Hi everyone,

another Rubber-Ducky gut-check question from me. I am learning about Enums and I
came up with a practice exercise to create a class that models a Soccer Player abstraction, this allows me to practice a number of concepts together including, basic class construction, Enums, e.t.c.

I wanted to ask if the class I am creating is too big or is becoming too bloated?

// This is the header file for the class (Footballer.hpp)

#include <iostream>
#include <string>

using namespace std;

class Footballer
{
    //Data Members
    //--------------

// I made these data members private because from my limited understanding 
// they fit the criteria of being invariant.
    private: 
        string dateOfBirth;                                                                         
        string name; 
    
// Justification: I used an Enum here because
// a soccer player can only play one of 4 positions on the field
// I felt this would be best represented as an Enum. 
    public: 
        enum struct PlayerPosition
        {
            Goalkeeper,
            Defender,
            Midfielder
            Striker
        };

// I made these data members public because I envisaged a scenario where
// they would need to be accessed by other functions and are subject to 
// change, an example is a soccer player may retire in the game after a 
// a certain age. And a players age is subject to change each year. 
        string club;		
        int playerAge; 

/*
   Here is where I have the question in regards to size of the class, my 
   gut feeling is that I could bundle these physical attributes of player 
   into a separate Struct and somehow have them connect to the Footballer 
   Class. 
*/                                                                             
        int balance;                                                                                 
        int speed; 
        int stamina;
        float weight;
        float height; 
        int strength; 

    // Member Functions 
    
    //----Constructor(s)------

    Footballer();                                                                                   
};

Any help, is humbly appreciated.

[jonnin]

I would move the enum out of the class, but no, its not that bloated. That enum 'feels' like it might be useful out of the class, eg if you had a container of players and wanted to filter it to only show the midfielders... you would want the enum out in the public... (it can be done either way, of course)? I dunno, it depends on how you use it whether it matters.

one wonders how you quantify some of those fields into numerics though... strength? what does 10 mean? If its for a game, sure, those can have values, but if its for real people, it seems odd.

also may want to rethink what you make public vs private. Some of that public data seems like it should not be, but here again, if you are just playing it saves time to make everything public when learning.

[ExponentialP]

Hi Jonnin!

Thanks for the reply do you think I should move the Enum outside side of the class so that it's like "global"? I took my lead from games like Fifa and Pro Evolution Soccer who quantify most of their player attributes as numerics, in my mind I sort of implement a scale of between 1 - 10 with 10 being to the top level of strength for example.

Thanks again for the reply and settling my anxiety around it, I guess I haven't seen what a truly bloated class looks like then lol.

[jonnin]

enums are not variables, they are constants, so the 'global variable' problem is not present. (I stick to the math idea that a constant is not a variable, and is in fact a type of opposite). In a large program, the enum should have its own namespace, but for now, you can skip that idea. It just protects you from name collisions in other name spaces. I put all constants and enums and the like in a namespace in a serious program, but the namespace itself is at the global scope.

Yes, I would put it out of the class, so you can have say a container of guys that you can then ask 'who is the goalkeeper' using that enum. If its in the class, you can still do it, but will there be times at the container level where having to reach into the class is awkward or causes work arounds? Dunno yet, and we may never see this if you don't build a huge wad of code. But its what you want to think about as you decide where to put the enum. It isn't 'wrong' inside, if that solves the problems you need solved. Its only 'wrong' if it keeps you from writing clean code later. But if you think about that now (and experience will eventually let you get a better feel for this stuff) you can give it your best shot. All I am saying is, 'think about it'. What you gonna do with that enum? That is the question. From the answer, you can take a solid stab at where to put it.

games makes sense; as I said, you can quantify things on virtual players but less so on humans, so I wanted to see if you had a clear idea of what you were building here. So that tells me you are building a piece of a game (that may never exist, fine, its practice).

for now stick to the idea of 'what this class represents should be all that is in it' and if you do that, it should not be bloated. if it starts feeling fat or bloated, find some common things, like your person's playing stats vs the identity (name, etc) info and make 2 sub classes to factor it out. Its kinda zen-ish as to how big something needs to be before you do that. I don't think you are there yet, but it may be good practice if you want to go ahead and do it.

FYI: age and DOB are redundant. watch for silly things like that.

[dhayden]

You have defined the PlayerPosition enum, but you don't have a member variable of that type to tell which position the player actually plays.

You should encode the data in a way that makes it "hard to get it wrong." What I mean is that it should be difficult for a developer to enter an invalid value. To that end, I suggest that a string for dateOfbirth is a bad idea. What if the someone enters "Penelope" as the date of birth? The program will happily store that. it would be better to store it as ints for month/day/year or a single int encoded as YYYYMMDD. Those can still be wrong, but they're better.

Age should be a method that computes the current age from the DOB, or perhaps you give it a date and the method computes the age on that date.

Don't worry too much about your initial draft of the class. it will evolve as you write the code. A fellow professional coder once wisely said to me that it takes 2 complete rewrites (i.e., three full versions) before code is really right.

[ExponentialP]

Hi all,

thank you both for those insights and points of best practice, I really can't express how much nudges like that help.