C++

Forum

 
Re-write code to use inheritance

wirelesskill (88)
Hi all,
So my assignment is to create 3 classes: Text, Box and TextBox.
Text has several values (font, size, color and data).
Box has similar values (width, hieght, border_color).
And TextBox has only two functions, SetTextBox and PrintTextBox.

The assignment is done, but my teacher has, maarked the work and essentially Ive done it wrong, it needs to use inheritance rather than composition. So Im a bit stuck at the moment as Im unsure as to whats the easiest way to change this now.

Heres my code:
main.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
 
#include <iostream>
#include "Text.h"
#include "Box.h"
#include "TextBox.h"

using namespace std;

int main(){

    Text text1;
    text1.SetText("Arial", 12, "Black", "Test text"); //String, int, string, string
    cout <<"Parameters are: " <<endl;
    text1.PrintText();
    cout << "\n";

    Box box1;
    box1.SetBox(15, 15, "Red");
    cout << "Box parameters: " <<endl;
    box1.PrintBox();
    cout << "\n";

    cout <<"Text and box put together:" <<endl;
    TextBox example;
    piemers.SetTextBox(text1, box1);
    piemers.PrintTextBox();

    return 0;
}


Text.h:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
#ifndef TEXT_H
#define TEXT_H
using namespace std;

class Text{

    public:
        Text();
        void SetText(string font, int size,  string color, string data);

        string GetFont();
        string GetColor();
        string GetData();
        int GetSize();

        void PrintText();

    private:
        string font;
        int size;
        string color;
        string data;
};

#endif // TEXT_H


text.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
 
#include <iostream>
#include "Text.h"
#include "Box.h"
#include "TextBox.h"

using namespace std;

Text::Text(){
}

void Text::SetText(string font, int size,  string color, string data){
    this->font = font;
    this->size = size;
    this->color = color;
    this->data = data;
}

string Text::GetFont(){
    return font;
}

string Text::GetColor(){
    return color;
}

string Text::GetData(){
    return data;
}

int Text::GetSize(){
    return size;
}

void Text::PrintText(){
    cout << "Font: " <<GetFont() <<endl;
    cout << "Size: " <<GetSize() <<endl;
    cout << "Color: " <<GetColor() <<endl;
    cout << "Data: " <<GetData() <<endl;
}


Box.h:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
 
#ifndef BOX_H
#define BOX_H
using namespace std;

class Box{

    public:
        Box();

        void SetBox(int width, int height, string box_color);

        int GetWidth();
        int GetHeight();
        string GetBorderColor();

        void PrintBox();

    private:
        int width;
        int height;
        string border_color;
};

#endif // BOX_H


box.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
 
#include <iostream>
#include "Text.h"
#include "Box.h"
#include "TextBox.h"

using namespace std;

Box::Box(){
}

void Box::SetBox(int width, int height, string box_color){
    this->width = width;
    this->height = height;
    this->border_color = box_color;
}

int Box::GetWidth(){
    return width;
}

int Box::GetHeight(){
    return height;
}

string Box::GetBorderColor(){
    return border_color;
}

void Box::PrintBox(){
    cout << "Width: " <<GetWidth() <<endl;
    cout << "Height: " <<GetHeight() <<endl;
    cout << "Box color: " <<GetBorderColor() <<endl;
}


TextBox.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
#ifndef TEXTBOX_H
#define TEXTBOX_H
using namespace std;


class TextBox : public Text, public Box{ /*This class would inherit everything from the other two classes and use 
their PrintText and PrintBox functions to be displayed in PrintTextBox.*/
    public:
        TextBox();
        void SetTextBox(const Text&, const Box&);/*Also, been told that this is incorrect if no copy constructor is described.
No idea what that means though???*/
        void PrintTextBox();

    private:
        Text text;
        Box box;
};

#endif // TEXTBOX_H


TextBox.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
#include <iostream>
#include "Text.h"
#include "Box.h"
#include "TextBox.h"

using namespace std;

TextBox::TextBox(){
}

void TextBox::SetTextBox(const Text& text, const Box& box){
    this->text = text;
    this->box = box;
}

void TextBox::PrintTextBox(){//At the moment, this does not work, it just displayes random numbers rather than the actual data
    PrintText();
    PrintBox();
}


I realise this is really lengthy and I apologise for that.

If anyone can point me in the right direction as to how to change this from compositions to inheritance based code, it would be greatly appreciated.

Repeater (3046)
I'm guessing you're meant to create a text class, containing font, size, color and data.

And create a Box class, containing width, height, border_color.

And then create a TextBox class that inherits from both of those, and adds functions SetTextBox and PrintTextBox.

Your textBox class inherits from text and box; good.
Your textBox class also contains a text and box; BAD.

This:
1
2
3
 
  private:
        Text text;
        Box box;

should not exist.

Your TextBox IS a text object. Why does it need to contain another text object?
Your TextBox IS a Box object. Why does it need to contain another Box object?
Last edited on
wirelesskill (88)
@Repeater
The thinking I had is that I have to create another object in order to display it in the PrintTextBox, which I realise now is wrong, I am simply inserting the already created objects into TextBox class..

What would SetTextBox method look like then?
1
2
3
4
 
void TextBox::SetTextBox(const Text& text, const Box& box){ //Right now its this.
    this->text = text;
    this->box = box;
}


Thanks
keskiverto (10402)
The explicit way:
1
2
3
4
5
6
7
8
9
 
void TextBox::SetTextBox(const Text& text, const Box& box) {
    this->font = text.font;
    this->size = text.size;
    this->color = text.color;
    this->data = text.data;
    this->width = box.width;
    this->height = box.height;
    this->border_color = box.box_color;
}

The "use base" approach:
1
2
3
4
 
void TextBox::SetTextBox(const Text& text, const Box& box) {
    this->setText( text );
    this->setBox( box );
}

Alas, your bases do not (quite) support that.
Last edited on
Repeater (3046)
1
2
 
this->text = text;
    this->box = box;


No. Your TextBox class SHOULD NOT contain an object of type text (so this->text should not exist), or an object of type box (so this->box should not exist).

Last edited on
JLBorges (13770)
See: http://www.cplusplus.com/forum/beginner/253088/#msg1113297
"this may be an exercise to understand multiple inheritance"

Just replace struct { with class { public: if the career teacher does not accept that structs are classes.
Ganado (6814)
Did you actually get marks off your grade for using composition? Because in your original thread, it was not clear that you had to use inheritance. Unless that was clearly stated in the requirements of the assignment, this seems rather unfair.

But now that we know that you need to use inheritance, I agree that you should revisit JLBorges' post.
wirelesskill (88)
Thanks for all the replies guys! Im revisiting and editing it now with all the suggestions.

@Genado Yup, our so-called career teacher is extremely ambiguous when it comes to assignments, essentially just tells us what he wants to see in terms of methods and classes and thats it. Then we have to come up with the rest which is probably why its so confusing at the moment. And now that he marked it we get another piece of the puzzle to try to solve this assignment. Luckily this is only one semester and he's gone the next :D
wirelesskill (88)
Hope someone can help me out again. Ive done this part of the assignment, but there was a first part which is doing the same thing, but using 3 different classes - Phone, CellPhone and SmartPhone.

The problem Im having is that he marked that work and has come back saying that I have not done it correctly, his notes:
1
2
3
 
wrong succession order, first base class is Phone. //Ive fixed this now, I had smartphone as base class
This requirement is not met - Derivatives classes must provide direct access to base class data /*But I do not understand this comment, 
and hope someone can point it out to me in my code?*/


Main.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
 
#include <iostream>
#include "Phone.h"
#include "CellPhone.h"
#include "SmartPhone.h"

using namespace std;

int main(){

    int weight;
    double battery;
    string OS;

            cout << "Enter phone weight: ";
            cin >> weight;
            cout <<endl;

    Phone samsung;

    samsung.setWeight(weight);
    samsung.PrintPhone();
    cout <<"\n";

            cout << "Enter battery size (mAh): ";
            cin >> battery;
            cout <<endl;

    CellPhone samsung1;

    samsung1.setBattery(battery);
    samsung1.PrintPhone();
    cout <<"\n";

            cout <<"Please enter the OS: ";
            cin >> OS;
            cout <<endl;

    SmartPhone samsung2;

    samsung2.setOS(OS);
    samsung2.PrintPhone();
    cout <<


    return 0;
}


Phone.h:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
#ifndef PHONE_H
#define PHONE_H

class Phone{

    public:
        Phone();
        void setWeight(int weight);
        int getWeight();
        void PrintPhone();

    private:
        int weight;
};

#endif // PHONE_H


Phone.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
#include <iostream>
#include "Phone.h"

using namespace std;

Phone::Phone(){
}

void Phone::setWeight (int weight){
    weight = weight;
}

int Phone::getWeight(){
  return weight;
}

void Phone::PrintPhone(){
    cout <<"Phone weight is: " <<getWeight() <<endl; /* I realise its silly to simply return the value that was just entered, but he wants to
see that we understand the concept.*/
}


CellPhone.h:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
#ifndef CELLPHONE_H
#define CELLPHONE_H

class CellPhone{

    public:
        Cellphone();
        void setBattery(int battery);
        double getBattery();
        void PrintPhone();

    private:
        double battery;

};

#endif // CELLPHONE_H


CellPhone.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
#include <iostream>
#include "CellPhone.h"

using namespace std;

CellPhone::Cellphone(){
}

void CellPhone::setBattery(int battery){
  battery = battery;
}

double CellPhone::getBattery(){
  return battery;
}

void CellPhone::PrintPhone(){
     cout <<"Phone battery size: " <<getBattery() <<endl;
}


SmartPhone.h:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
#ifndef SMARTPHONE_H
#define SMARTPHONE_H
#include "Phone.h"
#include "CellPhone.h"

using namespace std;

class SmartPhone : public Phone, public CellPhone{
    public:
        SmartPhone();
        void setOS(string OS);
        string getOS();

        void PrintPhone();

    private:
        string os;
};

#endif // SMARTPHONE_H


SmartPhone.cpp:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
#include <iostream>
#include "Phone.h"
#include "CellPhone.h"
#include "SmartPhone.h"
using namespace std;

SmartPhone::SmartPhone(){
}

void SmartPhone::setOS(string OS){
    os = OS;
}

string SmartPhone::getOS(){
    return os;
}

void SmartPhone::PrintPhone(){

        cout <<"Phone weight is: " <<getWeight() <<endl; /*And it appears Im now having some issues here, it is displaying some random numbers rather than the value that was entered. What am I doing wrong?*/

        cout <<"Phone battery is: " <<getBattery() <<" mAh." <<endl; //Same here.

        cout <<"Phone OS ir: " <<getOS() <<endl;

}


Youve all been amazing help with the other stuff above, and that is now working perfectly, so was hoping on some help with this one as well.
Last edited on
coder777 (8444)
Is there any reason that Phone isn't the base of CellPhone?
If it were only CellPhone would need to be the base of SmartPhone.


In main(): It doesn't make too much sense to have three objects for one phone. So you should have only one object for one phone, i.e. only SmartPhone samsung;.
wirelesskill (88)
I didnt see a need to have Phone as the base of CellPhone since it doesnt use its functions? Or am I wrong here?

Youre right! Not sure why I was using 3 seperate objects.. fixed that now.
Repeater (3046)
1
2
3
 
void Phone::setWeight (int weight){
    weight = weight;
}


This sort of thing really isn't helping. Stop giving your member function parameters the exact same name as class member variables. weight = weight; ?
coder777 (8444)
I didnt see a need to have Phone as the base of CellPhone since it doesnt use its functions? Or am I wrong here?
For inheritance it is not relevant whether the inherited class uses any members of the base class. Relevant is whether the inherited class extends the base class or not.

The question is 'is a' for inheritance or 'has a' for composite.

In your case CellPhone not is a Phone. And that's most likely your problem.

By the way: It is wise to initialize member variables that don't have a default constructor (suche as battery and weight) within you [default] constructor.

Newer version of C++ allow this:
1
2
3
4
5
6
7
8
9
10
11
 
class Phone{

    public:
        Phone();
        void setWeight(int weight);
        int getWeight();
        void PrintPhone();

    private:
        int weight = 0; // Now it is always initialized to 0 unless otherwise initialized
};


For inheritance see:
https://en.wikipedia.org/wiki/Inheritance_(object-oriented_programming)
wirelesskill (88)
@Repeater I knew this would come up hah, I dont know why but it makes it easier for me to visualise the links within the class. I know its a bad habit and I will work on it.

@Coder777 I have made your suggested changed and indeed it fixes the issues Im having. Thank you for the explanation on this.
Repeater (3046)
I dont know why but it makes it easier for me to visualise the links within the class.


That makes no sense. There is no inherent link between a parameter and the internals of your class.

Compare this:
1
2
3
 
void Phone::setWeight (int weight){
    weight = weight;
}


with 
1
2
3
4
 
void Phone::setWeight (int newWeight)
{
    weight = newWeight;
}

or 
1
2
3
4
 
void Phone::setWeight (int weightToSet)
{
    weight = weightToSet;
}


So much clearer.
Last edited on
Ganado (6814)
I don't think it's been explicitly stated yet... the root issue here isn't just clarity.
wirelesskill, weight = weight; is simply wrong. You're assigning the local variable to itself, and not touching the class member variable.
Last edited on
wirelesskill (88)
"Links" might have been the wrong word to use.

But yeah, I see what you're saying, I was completely messing it all up, I did see that and changed the names to weightNew just a little while ago. (Also a big headache when it came to errors...)

But everything seems in the right place now as everything is working as expected.
ne555 (10692)
this->weight = weight;

perhaps you should use the base class implementation of `PrintPhone()' instead of repeat it in the child
1
2
3
4
 
void SmartPhone::PrintPhone(){
   CellPhone::PrintPhone();
   out <<"Phone OS ir: " <<getOS() <<endl;
}
also, ¿have you seen virtual functions?


> Derivatives classes must provide direct access to base class data
not sure what he mean by this
your base class have getters/setters to expose its members, and you have public inheritance, so you get those getters/setters in the derivative too, so it's fully in the nude, ¿what more?

> our so-called career teacher is extremely ambiguous when it comes to assignments,
> we have to come up with the rest which is probably why its so confusing at the moment
talk to him. if you don't understand the assignment, you won't benefit from doing it
Topic archived. No new replies allowed.