Get/Set Functions

[alifarlig]

Im trying to develop a set of classes that represent some basic GUI elements that are intractable using a mouse or a keyboard. I have a problem with 2 of my classes.

*Label: labels are plain text controls. One should be able to change the text and color of a label through get/set functions.

class Label
{
private:
    char text;
    char color;

public:
    Label();

    void SetText(int x);
    void SetColor(int x);
    int GetColor();
    int GetText();
    ~Label();
};

Label::Label()
{
}

int Label::GetText()
{

    return text;
}

void Label::SetText(int x)
{
    text = x;
}

int Label::GetColor()
{

    return color;
}

void Label::SetColor(int x)
{
    color = x;

}



Label::~Label()
{
}

This is where Im at. I don't really understand how to develop the get/set functions so that Im able to change it.

Other class I got a problem with is:

*Imagebox: It is a simple obejct that can contain and draw an image inside it.

Dont even know where to start with this one.

[Elkvis]

Why is your text field using an int for its getter and setter? Shouldn't it be a string? Shouldn't it be stored as std::string, rather than char?

[alifarlig]

Why is your text field using an int for its getter and setter? Shouldn't it be a string? Shouldn't it be stored as std::string, rather than char?

Like this?

private:
std::string text;
char color;

void SetText(string x);

string GetText();


What about color? any thoughts?

[Elkvis]

Color can be an int. Colors are often represented as 24-bit or 32-bit values, with groups of 8-bits representing the red, green, blue, and alpha channels. You should look into passing string arguments by (const) reference, rather than by value, which you are doing here.

[kmdv]

Color can be an int. Colors are often represented as 24-bit or 32-bit values, with groups of 8-bits representing the red, green, blue, and alpha channels. You should look into passing string arguments by (const) reference, rather than by value, which you are doing here.

I would definitely not recommend using a bare int for something like color. It would be better to wrap that int with a Color class. That way there is no need to care about the underlying format (whether its RGB, ARGB, whatever).

[alifarlig]

I would definitely not recommend using a bare int for something like color. It would be better to wrap that int with a Color class. That way there is no need to care about the underlying format (whether its RGB, ARGB, whatever).

What if I change it like this? Im not 100% sure how I should initilize it in the constructor. And im not sure what to do with my "DrawString" class.

class Label
{
private:
    std::string text;


public:
    Label();
    Label(string text,int x, int y);



    struct rgb
    {
        int r;
        int g;
        int b;
    };

    rgb textColor;

    void DrawString(string text, int x, int y);
    void SetText(string Ntext) { Ntext = text; }
    void SetColor(int R,int G, int B) { textColor.r = R; textColor.g = G; textColor.b = B; }
    rgb GetColor() { return textColor; };
    string GetText() { return text; }
    ~Label();
};

Label::Label()
{
}

Label::Label(string text,int x, int y)
{

}

void Label::DrawString(string text, int x, int y)
{

}

string Label::GetText() {return text;}


void Label::SetText(string Ntext) { Ntext = text; }



Label::rgb Label::GetColor() { return textColor; }


void Label::SetColor(int R, int G, int B) { textColor.r = R; textColor.g = G; textColor.b = B; }

[Guest]

It's probably sufficient to use a byte / char (that's 256 different values) for each color in your program.

You can initialize class members in your constructor using initializer lists.

You're on a good way, but there's definitely room for improvement, so keep at it

[kmdv]

What if I change it like this? Im not 100% sure how I should initilize it in the constructor. And im not sure what to do with my "DrawString" class.

class Label
{
private:
    std::string text;


public:
    Label();
    Label(string text,int x, int y);



    struct rgb
    {
        int r;
        int g;
        int b;
    };

    rgb textColor;

    void DrawString(string text, int x, int y);
    void SetText(string Ntext) { Ntext = text; }
    void SetColor(int R,int G, int B) { textColor.r = R; textColor.g = G; textColor.b = B; }
    rgb GetColor() { return textColor; };
    string GetText() { return text; }
    ~Label();
};

Label::Label()
{
}

Label::Label(string text,int x, int y)
{

}

void Label::DrawString(string text, int x, int y)
{

}

string Label::GetText() {return text;}


void Label::SetText(string Ntext) { Ntext = text; }



Label::rgb Label::GetColor() { return textColor; }


void Label::SetColor(int R, int G, int B) { textColor.r = R; textColor.g = G; textColor.b = B; }

You are heading in the right direction, but:
- This type does not look like specific to Label only, so it should be moved outside of the label to be more reusable (you don't want to create new RGB type for each of your controls).
- The name implies that it is stored in RGB. I would rename this class simply to Color.
- I would use getters instead of exposing the fields directly. Something like internal color representation (floats vs RGB int vs ARGB int vs bytes) is something that might change in the future. Therefore, it is better to encapsulate it now.

class Color
{
public:

    explicit Color(unsigned int red, unsigned int green, unsigned int blue) :
        red(red),
        green(green),
        blue(blue)
    {
    }
    
    unsigned int getRed() const
    {
        return red;
    }
    
    unsigned int getGreen() const
    {
        return green;
    }
    
    unsigned int getBlue() const
    {
        return blue;
    }
        
private:
    
    unsigned char red;
    unsigned char green;
    unsigned char blue;
};

Also, your label's SetColor() could be just SetColor(Color color) instead of SetColor(r, g, b).