Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm currently thinking about an interface to a class I'm writing. This class contains styles for a character, for example whether the character is bold, italic, underlined, etc. I've been debating with myself for two days whether I should use getters/setters or logical names for the methods which change the values to these styles. While I tend to prefer logical names, it does mean writing code that is not as efficient and not as logical. Let me give you an example.

I've got a class CharacterStyles which has member variables bold, italic, underline (and some others, but I'll leave them out to keep it simple). The easiest way to allow other parts of the program to access these variables, would be to write getter/setter methods, so that you can do styles.setBold(true) and styles.setItalic(false).

But I do not like this. Not only because a lot of people say that getters/setters break encapsulation (is it really that bad?), but mostly because it doesn't seem logical to me. I expect to style a character through one method, styles.format("bold", true) or something like that, but not through all these methods.

There is one problem though. Since you can't access an object member variable by the contents of a string in C++, I would either have to write a big if-statement/switch container for all styles, or I would have to store the styles in an associative array (map).

I can't figure out what the best way is. One moment I think I should write the getters/setters, and the next moment I lean towards the other way. My question is: what would you do? And why would you do that?

share|improve this question
    
Does the CharacterStyles class actually do anything other than bundle three booleans? How does it get consumed? –  Mark Canlas Aug 24 '12 at 21:37
    
Yes, because CharacterStyles should be able to inherit from other CharacterStyles. This means that if I have a CharacterStyles with bold set to true and the other variables undefined, the getter methods for the other variables should return the value of the parent style (which is stored in another property), while the getter for the bold property should return true. There are some other things like a name and the way it's displayed in the interface as well. –  Frog Aug 24 '12 at 21:41
    
You could use an enum to define the various style options, then use a switch statement. Btw, how are you handling undefined? –  Tyanna Aug 24 '12 at 21:54
    
@Tyanna: Indeed, I also thought about using an enum, but that's just a minor detail. I was planning to just set undefined values to NULL. Isn't that the best way? –  Frog Aug 24 '12 at 22:09

4 Answers 4

up vote 4 down vote accepted

Yes, getters/setters do break encapsulation - they basically are just an extra layer between directly accessing the underlying field. You might as well just access it directly.

Now, if you want more complex methods to access the field, that's valid, but instead of exposing the field, you need to think what methods the class should be offering instead. ie. instead of a Bank class with a Money value that is exposed using a property, you need to think what types of access a Bank object should offer (add money, withdraw, get balance) and implement those instead. A property on the Money variable is only syntactically different from exposing the Money variable directly.

DrDobbs has an article that says more.

For your problem, I'd have 2 methods setStyle and clearStyle (or whatever) that take an enum of the possible styles. A switch statement inside these methods would then apply the relevant values to the appropriate class variables. This way, you can change the internal representation of the styles to something else if you later decide to store them as a string (for use in HTML for example) - something that would require all users of your class to be changed too if you used get/set properties.

You can still switch on strings if you want to take arbitrary values, either have a big if-then statement (if there's a few of them), or a map of string values to method pointers using std::mem_fun (or std::function) so "bold" would be stored in a map key with its value being a sts::mem_fun to a method that sets the variable bold to true (if the strings are the same as the member variable names, then you can also use the stringifying macro to reduce the amount of code you need to write)

share|improve this answer
    
Excellent answer! Especially the part about storing the data as HTML struck me as a good reason to indeed go down this road. So you would suggest storing the different styles in an enum and then setting styles like styles.setStyle(BOLD, true)? Is that correct? –  Frog Aug 25 '12 at 19:37
    
yes, only I'd have 2 methods - setStyle(BOLD) and clearstyle(BOLD). forget the 2nd parameter, I just prefer it like that and you could then overload clearStyle to take no parameters to clear all the style flags. –  gbjbaanb Aug 26 '12 at 13:52
    
That wouldn't work, because some types (like font-size) need a parameter. But still thanks for the answer! –  Frog Aug 26 '12 at 14:45
    
I've been trying to implement this, but there's one problem I didn't consider: how do you implement styles.getStyle(BOLD) when you do not only have member variables of type boolean, but also of types integer and string (for example: styles.getStyle(FONTSIZE)). Since you can't overload return types, how do you program this? I know you could use void pointers, but that sounds like a really bad way to me. Do you have any hints on how to do this? –  Frog Aug 26 '12 at 18:24
    
you could return a union, or a struct, or with the new standard, you can overload by return type. That said, are you trying to implement a single method to set a style, where style is a flag, and a font-family, and a size? Maybe you're trying to force the abstraction a little too much in this case. –  gbjbaanb Aug 26 '12 at 22:49

I think you are over-thinking the issue.

styles.setBold(true) and styles.setItalic(false) are OK

share|improve this answer

One idea you might not have considered is the Decorator Pattern. Rather than setting flags in an object and then applying those flags to whatever you're writing, you wrap the class that does the writing in Decorators, which in turn apply the styles.

The calling code doesn't need to know how many of these wrappers you've put around your text, you just call a method on the outer object and it calls down the stack.

For a pseudocode example:

class TextWriter : TextDrawingInterface {
    public:
        void WriteString(string x) {
            // write some text somewhere somehow
        }
}

class BoldDecorator : TextDrawingInterface {
    public:
        void WriteString(string x) {
            // bold application on
            m_textWriter.WriteString(x);
            // bold application off
        }

        ctor (TextDrawingInterface textWriter) {
            m_textWriter = textWriter;
        }

    private:
        TextWriter m_TextWriter;
}

And so on, for each decorating style. In its simplest usage, you can then say

TextDrawingInterface GetDecoratedTextWriter() {
    return new BoldDecorator(new ItalicDecorator(new TextWriter()));
}

And the code that calls this method doesn't need to know the detail of what it's receiving. As long as it's SOMETHING that can draw text through a WriteString method.

share|improve this answer
    
Hmmm, I'm going to take a look at that tomorrow with a fresh mind and will get back to you then. Thanks for the answer. –  Frog Aug 24 '12 at 22:11
    
I'm a big fan of the Decorator pattern. The fact that this pattern resembles HTML (where the core operation is to enclose the input string with tags) is an attestation that this approach can satisfy all of your interface user's needs. One thing to keep in mind is that an interface that is good and easy-to-use, may have an underlying implementation that is technically complicated. For example, if you are actually implementing the text renderer yourself, you may find that TextWriter needs to talk to both BoldDecorator and ItalicDecorator (bi-directionally) to get the job done. –  rwong Aug 25 '12 at 1:03
    
while this is a nice answer, does it not re-introduce the op's question? "bold applicaton on" -> how is this going to be done? using setters/getters like SetBold()? Which is exactly the op's question. –  stijn Aug 25 '12 at 13:36
1  
@stijn: Not really. There are a number of ways of avoiding that situation. For example, you can wrap that in a builder with a toggleStyle method (enum argument) which adds and removes decorators from an array, only decorating the final item when you call BuildDecoratedTextWriter. Or you can do as rwong suggested and complicate the base class. Depends on the circumstance and the OP wasn't specific enough about the overall spec to guess as to which is best for him. He seems smart enough to be able to figure it out once he gets the pattern though. –  pdr Aug 25 '12 at 17:18
1  
I've always thought the Decorator Pattern implies an ordering to the decorations. Consider the example code shown; how does one go about removing the ItalicDecorator? Still, I think something like Decorator is the way to go. Bold, italic, and underline are not the only three styles. There's thin, semibold, condensed, black, wide, italic-with-swash, small caps, etc. You may need a way to apply an arbitrarily large set of styles to a character. –  Barry Brown Aug 25 '12 at 22:25

I'd lean towards the second solution. It looks more elegant and flexible. Although it's difficult to imagine other types than bold, italic and underline (overline?), it would be difficult to add new types using member variables.

I used this approach in one of my latest projects. I have a class which can have multiple boolean attributes. The number and names of the attributes may change in time. I store them in a dictionary. If an attribute is not present I assume its value is "false". I also have to store a list of available attribute names, but that's another story.

share|improve this answer
1  
Apart from these types I would add strikethrough, font-size, font-family, superscript, subscript, and maybe others later on. But why did you chose for that method in your project? Don't you think it's dirty code to have to store a list of allowed attributes? I'm really curious what your answers are to these questions! –  Frog Aug 24 '12 at 22:14
    
I had to deal with situation where some attributes could be defined by clients when the application was already deployed. Some other attributes were required for some clients but obsolete to the others. I could customize the forms, the stored data sets and address other issues using this approach. I don't think it generates dirty code. Sometimes you just can't predict what kind of information will be needed by your customer. –  Andrzej Bobak Aug 25 '12 at 8:17
    
But my customer won't need to add custom attributes. And in that case I think it looks a bit "hacky". Don't you agree? –  Frog Aug 25 '12 at 8:47
    
If you don't face dynamic number of attributes issue you don't need flexible space to store them. That's true :) I don't agree with the other part though. Stroring attributes in a dictionary/hashmap/etc is not a bad approach in my opinion. –  Andrzej Bobak Aug 25 '12 at 11:05

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.