Class Design question.

Hi, I've got a class design problem. Well, actually it's not a big problem. I can somewhat make the class but I don't quite confidence that what I built was a good OOP practice.

I have two class for two different object. They have an integer as state. If one object changes the state, the other changed too. Here's what I've done:

Code:

---

class A
{
private:
  int mState;
public:
  inline void changeState(int state)
  {
    mState=state;
  }
};


class B
{
private:
  int mState;
public:
  inline void changeState(int state)
  {
    mState=state;
  }
};


class C
{
private:
  A mA;
  B mB;

public:
  void changeStateA(int state);
  void changeStateB(int state);
};

void C::changeStateA(int state);
{
    mA(state);
    switch(state)
    {
        1:
        4:
            mB(1);
            break;
        2:
        5:
            mB(1);
            break;
        3:
        6:
            mB(1);
            break;
    }
}


void C::changeStateB(int state);
{
    mB(state);
    mA(state);
}

---

And in the main class, I referenced the C class. Is it okay to do that? Is my code a good practice? Or is there any other better way to do that?

Thanks in advance.

Your code cannot possibly compile as it is. Your switch is a mess and is missing the case keyword. You keep using the mA and mB objects like functions even though they have no function call operator. I have no idea what you want to do.

Aside from that, I think you presented the problem in too abstract terms.

Quote:

---

Originally Posted by CornedBee

Your code cannot possibly compile as it is. Your switch is a mess and is missing the case keyword. You keep using the mA and mB objects like functions even though they have no function call operator. I have no idea what you want to do.

Aside from that, I think you presented the problem in too abstract terms.

---

Well, I didn't put all the functions and all the variables of the classes. Also I didn't type case to simplify the code (I'm just too lazy to type case over and over. it's past my working hour and I'm rather reluctant to do anything beside going to bed. :D). Okay so to put it bluntly, I'll change my question to: How do I make 2 objects from 2 different classes that can know each other's change of value?

> How do I make 2 objects from 2 different classes that can know each other's change of value?

One way is to provide each class with a public function that changes the value and calls the sister function on the other class.

After I had a look again into my application, I was more confused because my class design is pretty crappy and confusing even for me. Because I needed a specific answer, so I'll try to explain my situation here.

I have a GUI of keyboard buttons in the screen with SDL. The buttons are images. If I mouse clicked the area inside the button, the corresponding button in the screen blinks. The buttons of alphabet, month or numbers weren't showed at the same time. There's an enumeration state of ALPHA, MONTH and NUMS for the current one being showed and active in the screen. Beside that, it will add a char to a string data. The data themself are consisting of 4 input textbox with each input box declaring a spesific state (let's call it NAME, MONTH, DAY, and YEAR. In the object that handles the data, there's a bitmap font to show it in the screen. And the last object is a blinking cursor like in a text editor. So basicaly I'm trying to make a similar UI like MFC but in SDL.

Here's what I've done (BTW, I'll just show you only the declaration of methods because the implementation of the methods are too many to write. Let's just assume that they worked because, well, they were. ):

The class for keyboard buttons

Code:

---

class CKeyboard
{
private:
  std::map <char, CSprites *> mButtonsMap; //The map of buttons sprites
  int mState; //ALPHA, MONTH and NUM state;
public:
  CKeyboard(); //Constructor. This is were we instantiated the buttons and put it in the map.
  ~CKeyboard(); //Destructor
  char checkClickArea(int xx, int yy); //Checks the click area for a button. Returned the char of the button if successful, or NULL if there's no button in the area. The area depends on the current active state.
  void blinkButton(char button); //Blinking animation of the button 
  void setState(int state); //Set the active state
};

---

The class for character data

Code:

---

class CCharacterData
{
private:
  CBitmapFont *mFont; //The current bitmap font
  CSprites *mMonthSprite; //Image representation of the month
  std::string mName; /Name
  int mMonth; //Month: 1 - 12
  std::string mDay; //Age
  std::string mYear; //Job
  bool nameShow; //To set whether the first name string will be displayed in the screen
  bool monthShow; //To set whether the first name string will be displayed in the screen
  bool dayShow; //To set whether the first name string will be displayed in the screen
  bool jobShow; //To set whether the first name string will be displayed in the screen

  int mActiveField; //Set the current active record that can be inputted. The options are NAME, MONTH, DAY, and YEAR.
public:
  CCharacterData(CBitmapFont *font); //Constructor. Also set the current font
  ~CCharacter(); //Destructor
  void nameAdd(char theChar); //Add one char to name
  void nameDeleteLastChar(); //Delete the last char
  int nameGetPixelLength(); //Get the pixel length of the name string to determine the current position of the cursor in name state
  void monthPick(int month); //Pick the month
  void monthDelete(); //Month = 0
  int monthGetValue(); //Get the value of the month
  void dayAdd(char theChar); //Add one char to day
  void dayDeleteLastChar(); //Delete the last char
  int dayGetPixelLength(); //Get the pixel length of the day string to determine the current position of the cursor in day state
  void yearAdd(char theChar); //Add one char to year
  void yearDeleteLastChar(); //Delete the last char
  int yearGetPixelLength(); //Get the pixel length of the year string to determine the current position of the cursor in year state
  void setVisible(int record, bool state); //State the showing boolean of the corresponding record
  void setActiveField(int state); //Set the current active field
};

---

The cursor class:

Code:

---

class CCursor
{
private:
  CSprites *mCursor; //The sprites of a blinking cursor
  int mPosX; //Current X Position of the cursor
  int mPosY; //Current Y Position of the cursor

  const int mNamePosX; // name X Position constants
  const int mNamePosY; // name Y Position constants

  const int mMonthPosX; //month X Position constants
  const int mMonthPosY; //month Y Position constants

  const int mDayPosX; //day X Position constants
  const int mDayPosY; //day Y Position constants

  const int mYearPosX; //year X Position constants
  const int mYearPosY; //year Y Position constants

  int mState; //The current position state. The options are NAME, MONTH, DAY, and YEAR.
  bool mShow; //Boolean for showing / hiding cursor
public:
  CCursor(); //Constructor.
  ~CCursor(); //Destructor
    inline void setState(int state); /Set the current state
    inline int getState(); //Get the current state
    inline void setVisible(bool show); //Set show /hide cursor
    void moveCursor(int xx); //Move the cursor based on the current state of the cursor. If the state is NAME, the current x of the cursor will be (mNamePosX+xx) where xx = CharHandler->nameGetPixelLength() and the current y wil be mNamePosY, for month, in MONTH state the x and y position will be mMonthPosX and Y if =0, and invisible if !=0, etc.
};

---

In the main class:

Code:

---

class CMainApp
{
private:
  int mState; //Current active record that can be inputted. The options are NAME, MONTH, DAY, and YEAR.
  CKeyboard *mKeyb;
  CCharacterData *mCharData;
  CCursor *mCursor;
  bool done; //The flag to indicate the quit parameter of the main loop
public:
  CMainApp(); //Constructor
    ~CMainApp(); //Destructor
  void mainLoop(); //The main loop of the program
  void setState(int state); //Set the current state
  ....
};

void CMainApp::mainLoop()
{
  while (!done)
  {
    if (eventMouseClick())
    {
        char mInputChar;
        if(mInputChar=mKeyb->checkClickArea(getMousePos()->x,getMousePos()->y) != NULL)
        {
              switch(mState)
              {
                  case 1:
                      mCharData->nameAdd(mInputChar);
                      mCursor->moveCursor(mCharData->nameGetPixelLength());
                      break;

                  case 2:
                      mCharData->monthPick(atoi(mInputChar));
                      if (mCharData->monthGetValue()!=0)
                          mCursor->setVisible(true); //Set show
                      else
                          mCursor->setVisible(false); //Set hide
                      break;

                  case 3:
                      mCharData->dayAdd(mInputChar);
                      mCursor->moveCursor(mCharData->dayGetPixelLength());
                      break;

                  case 4:
                      mCharData->jobAdd(mInputChar);
                      mCursor->moveCursor(mCharData->yearGetPixelLength());
                      break;
        }
    }

    if (eventKeyPress())
    {
        if (getKey()==13)
        {
              if (mState!=4)
                  setState(mState+1);
              else
                  setState(1);
        }
    }
    draw(); //Draw with SDL

  }
}


void CMainApp::setState(int state)
{
  switch(state)
  {
    case 1:
        mKeyb->setState(1); //ALPHA
        mCharData->setActiveField(1); //NAME
        mCursor->setState(1); //NAME
        break;
    case 2:
        mKeyb->setState(2); //MONTH
        mCharData->setActiveField(2); //MONTH
        mCursor->setState(2); //MONTH
        break;
    case 3:
        mKeyb->setState(3); //NUM
        mCharData->setActiveField(3); //DAY
        mCursor->setState(3); //DAY
        break;
    case 4:
        mKeyb->setState(3); //NUM
        mCharData->setActiveField(4); //YEAR
        mCursor->setState(4); //YEAR
        break;
  }
}

---

As you can see my coding is pretty crappy. What I wanted to do is to make the char data, keyboard, and cursor classes to interact one another. What do you suggest to better my code? Any input will be appreciated. Thanks in advance.

It maybe just a syle issue, but most prefer to place public date before private and protected. It makes it stand out first, and then the varialbe delcatrions ( private ) ususally follow.

Code:

---

class Foo
{
public:
  Foo();
  ~Foo();
  int getData();
  void setData ( int );

private:
int data;
};

---

Ok, because there's no reply whatsoever, I assumed that my shabby code is the best way to do it. So I'll try to stick with it. Thanks.

The problem is that your dumping too much code, mate. It takes some time to look it over and honestly some of us may not feel inclined wasting too much time on a post. It all depends on how we are feeling that day.

Instead of wanting to have all of your problem solved in one go, if you manage to divide it into more digestible parts, it's probably easier to move on from there.

Also, when posting code (especially when posting so much code) it's best to drop most of the comments. You have no idea how hard is to read code when the average number of charaters per line is probably close to 60.

Quote:

---

Originally Posted by Mario F.

The problem is that your dumping too much code, mate. It takes some time to look it over and honestly some of us may not feel inclined wasting too much time on a post. It all depends on how we are feeling that day.

Instead of wanting to have all of your problem solved in one go, if you manage to divide it into more digestible parts, it's probably easier to move on from there.

Also, when posting code (especially when posting so much code) it's best to drop most of the comments. You have no idea how hard is to read code when the average number of charaters per line is probably close to 60.

---

Well, sorry about that. But that's as short as I can go. The actual ones are actually alot bigger than what I've posted here. The comments themself are there because I didn't include the actual implementation of the methods cause it will make it a lot more codes. I can remove all the comments if you like but then you'll have to figure out what each methods do. And seeing as my problem is not in a particular line in the code, but rather a class design or OOP issue, the code itself would be consisting of lots of classes. Again sorry about that.

Right now the only advise I can give you is that generally, if you believe your hierarchy is too complicated, than it is too complicated.

There's a fine line between who benefits most from the code; you that need to write it, or the program that needs it to run. Finding that point where just enough complexity was added for the code so that it runs as you want it to run and yet you can still read it and maintain it, is not easy. Master cooks experience something similar when trying to melt sugar into caramel.

However it is easier to ascertain when you overdid it. When you added too much complexity. It's when you suddenly realize you can't read your own code. This seems to be your case. You are starting to feel overwhelmed by it. Its time you stop and reevaluate.

And now comes the hard part. These are the questions you should ask yourself and answer thrutfully to yourself:

- Can you do what you want to do? Do you have the knowledge? Because more often than not, complexity is added exactly because we don't know the solution and run circles around it.

- Do you need to do it? Is this an imperative? Because if it's too complicated maybe it's best if you try small first. Simpler projects, adapting your brain and slowly training it to grasp more complex structures and not lose track of them. It is a strong undeniable fact that we need to train our brain in C++ as much as we need to teach it about C++.

Finally, OO design advise (the one you are asking) is perhaps one of the hardest to give. I have an open thread on the game forums on this exact same issue, of which I'm ready to accept a minimal set of answer, if any, and minimal new knowledge with them. It's not that people don't know. It's more that there's is no right answer on most cases. OOP implementation is somewhat simple. We have virtual functions and inheritance. From these two concepts we expand to polymorphism... and guess what? That's OOP. That's it. Nothing more.

How we set relations between objects, if we create a generalization or composition, if we create an object or decide to implement it as members of an already existing object,... this and many other decisions are made after a thought process. Some times a very long thought process. This is what makes good OOP so hard. And either someone experienced your exact same problem, or they will be forced to think hard on it in order to give a satisfactory answer.

Of course experience counts. Some folks here can certainly burn much less brais cells and detect a pattern in your requirements similar to something they did already or they read about. But that's when presenting your problem in a more symplistic way helps. It will catch their attention.

We don't need your class definitions. Every member and function. We need your current classes names, their purposes, and we need your requirements. All delineated in the most simple way you can. :)

>>- Can you do what you want to do? Do you have the knowledge? Because more often than not, complexity is added exactly because we don't know the solution and run circles around it.

Truthfully speaking, no I don't think so. I created them classes, inheritances, references and whatnot "on the fly", if you know what I mean. I created them as I see fit as I did these classes. And when I looked back at them again, I think it can be simplified more, just don't know how to do it.

>>- Do you need to do it? Is this an imperative? Because if it's too complicated maybe it's best if you try small first. Simpler projects, adapting your brain and slowly training it to grasp more complex structures and not lose track of them. It is a strong undeniable fact that we need to train our brain in C++ as much as we need to teach it about C++.

Actually maybe no. The code worked fine as it is. But in the long run it's too difficult to re-engineer or re-develop. I've made simpler codes before with some pretty good OOP practice. But in a big and more complex project like what I do right now, I get confused to create the design. Should I use inheritance here? Should I seperate this big bulky class to seperate classes? Etc.

>>Finally, OO design advise (the one you are asking) is perhaps one of the hardest to give. I have an open thread on the game forums on this exact same issue, of which I'm ready to accept a minimal set of answer, if any, and minimal new knowledge with them. It's not that people don't know. It's more that there's is no right answer on most cases. OOP implementation is somewhat simple. We have virtual functions and inheritance. From these two concepts we expand to polymorphism... and guess what? That's OOP. That's it. Nothing more.

Yeah I know that already. It's just that I think my OO design is just in the bottom of the good OOP list.

>>How we set relations between objects, if we create a generalization or composition, if we create an object or decide to implement it as members of an already existing object,... this and many other decisions are made after a thought process. Some times a very long thought process. This is what makes good OOP so hard. And either someone experienced your exact same problem, or they will be forced to think hard on it in order to give a satisfactory answer.

Right again. Been experiencing this more as the code grew.

>>Of course experience counts. Some folks here can certainly burn much less brais cells and detect a pattern in your requirements similar to something they did already or they read about. But that's when presenting your problem in a more symplistic way helps. It will catch their attention.

I know that that's one thing that I lack. Experience. Sadly in my current project I was let loose by the boss. If you have a problem deal with it yourself, said he. He only give me the requirement.

>>We don't need your class definitions. Every member and function. We need your current classes names, their purposes, and we need your requirements. All delineated in the most simple way you can. :)

Okay I'll try to make it into simple sentences.

There's three classes referenced in the main class. The three of them and the main class has an enum as a state. In the main class, CCharData class, and CCursor class there's NAME, MONTH, DAY, and YEAR states. In the CKeyboard class there's ALPHABET, MONTH, and NUM states. If there's a key press event in the main loop in main class, this will happen:

Code:

---

void CMainApp::mainLoop()
{
    .......
    if (eventKeyPress())
    {
        if (getKey()==13)
        {
              if (mState!=4)
                  setState(mState+1);
              else
                  setState(1);
        }
    }


  }
}


void CMainApp::setState(int state)
{
  mState=state;
  switch(state)
  {
    case 1:
        mKeyb->setState(1); //ALPHA
        mCharData->setActiveField(1); //NAME
        mCursor->setState(1); //NAME
        break;
    case 2:
        mKeyb->setState(2); //MONTH
        mCharData->setActiveField(2); //MONTH
        mCursor->setState(2); //MONTH
        break;
    case 3:
        mKeyb->setState(3); //NUM
        mCharData->setActiveField(3); //DAY
        mCursor->setState(3); //DAY
        break;
    case 4:
        mKeyb->setState(3); //NUM
        mCharData->setActiveField(4); //YEAR
        mCursor->setState(4); //YEAR
        break;
  }
}

---

So if the state in main class changed, the other classes' state changed also. In CKeyboard class, there's a function to check whether the mouse click coordinate is inside the button sprite or not. If it is, then it will return a char value of the respective button or NULL if not. The return value will be added to the CCharData's string value of the current state's variable. Then the string will be blitted to the screen with a bitmap font system. When the current string was added we also moved the CCursor's position based on the pixel width of the bitmap string and blited it to the screen. Is this simpler than before?

Thanks alot.