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?
4 Answers 4
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)
-
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– Frog08/25/2012 19:37:14Commented Aug 25, 2012 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– gbjbaanb08/26/2012 13:52:06Commented Aug 26, 2012 at 13:52
-
That wouldn't work, because some types (like font-size) need a parameter. But still thanks for the answer!Frog– Frog08/26/2012 14:45:29Commented Aug 26, 2012 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– Frog08/26/2012 18:24:52Commented Aug 26, 2012 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– gbjbaanb08/26/2012 22:49:33Commented Aug 26, 2012 at 22:49
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.
-
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– Frog08/24/2012 22:11:25Commented Aug 24, 2012 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 bothBoldDecorator
andItalicDecorator
(bi-directionally) to get the job done.rwong– rwong08/25/2012 01:03:59Commented Aug 25, 2012 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– stijn08/25/2012 13:36:28Commented Aug 25, 2012 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– pdr08/25/2012 17:18:01Commented Aug 25, 2012 at 17:18
-
1I'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– Barry Brown08/25/2012 22:25:53Commented Aug 25, 2012 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.
-
1Apart 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– Frog08/24/2012 22:14:50Commented Aug 24, 2012 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– Andrzej Bobak08/25/2012 08:17:17Commented Aug 25, 2012 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– Frog08/25/2012 08:47:56Commented Aug 25, 2012 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– Andrzej Bobak08/25/2012 11:05:52Commented Aug 25, 2012 at 11:05
I think you are over-thinking the issue.
styles.setBold(true)
and styles.setItalic(false)
are OK
bold
set totrue
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 thebold
property should returntrue
. There are some other things like a name and the way it's displayed in the interface as well.