7

I've been trying to design an interface for a data class I'm writing. This class stores styles for characters, for example whether the character is bold, italic or underlined. But also the font-size and the font-family. So it has different types of member variables. The easiest way to implement this would be to add getters and setters for every member variable, but this just feels wrong to me. It feels way more logical (and more OOP) to call style.format(BOLD, true) instead of style.setBold(true). So to use logical methods instead of getters/setters.

But I am facing two problems while implementing these methods: I would need a big switch statement with all member variables, since you can't access a variable by the contents of a string in C++. Moreover, you can't overload by return type, which means you can't write one getter like style.getFormatting(BOLD) (I know there are some tricks to do this, but these don't allow for parameters, which I would obviously need).

However, if I would implement getters and setters, there are also issues. I would have to duplicate quite some code because styles can also have a parent styles, which means the getters have to look not only at the member variables of this style, but also at the variables of the parent styles.

Because I wasn't able to figure out how to do this, I decided to ask a question a couple of weeks ago. See Object Oriented Programming: getters/setters or logical names. But in that question I didn't stress it would be just a data object and that I'm not making a text rendering engine, which was the reason one of the people that answered suggested I ask another question while making that clear (because his solution, the decorator pattern, isn't suitable for my problem). So please note that I'm not creating my own text rendering engine, I just use these classes to store data.

Because I still haven't been able to find a solution to this problem I'd like to ask this question again: how would you design a styles class like this? And why would you do that?

5
  • 1
    I think that style.setBold(true) is better than style.format(BOLD, true). Also, you could consider variations such as Style.FontStyle=true and Style.FontItalic=False. You may also want to follow an existing similar system as in .NET in RichTextBox or System.Drawing name space: msdn.microsoft.com/en-us/library/system.drawing.font.bold
    – NoChance
    Sep 9, 2012 at 12:28
  • @EmmadKareem: I'm afraid I can't do that, because in C++ Style.FontStyle = true would mean I'd have to make FontStyle a public variable, which is not possible because it should return the style of the parent if the member variable is NULL.
    – Frog
    Sep 9, 2012 at 12:41
  • 2
    @Frog: not exactly true. You could define an Attribute<T> class that overloaded operator== and also allowed conversion to T. Jun 18, 2013 at 7:26
  • @kevincline: Good idea! A shame I already finished this project, but something I'll keep in mind next time I need something like this. Thanks!
    – Frog
    Jun 20, 2013 at 13:46
  • 1
    @Frog: before you create any general-purpose class like Attribute<T> check to see if there is something similar at boost.org Jun 20, 2013 at 19:50

3 Answers 3

9

Part 1

This is a good design question. You are correct in detecting code smell concerning getters and setters. They generally indicate a design problem exposing the implementation details of your object.

Try to think in terms of what your objects should do - Tell, Don't Ask:

Your first problem may be that you are trying to design "data classes". Rather than worry about the data (implementation details), think about the functionality. Again, what should your objects do? In your case, what do you want to do with character styles? Who (software-wise) cares about character styles? What do they need to do?

Hopefully that gets you started. Test driven development helps with these kinds of design problems. It forces you to think in terms of function not data.

On the contrary, if all you need is a data container, write a C-style struct class and go to town. I wouldn't recommend that, maintaining it will be a $@#%#.

Good luck!

Part 2

What you want is either a simple data class - skip the getters and setters completely, or you want to create a platform independent abstraction that places a facade (Facade Pattern) in front of the rendering and style setting. It simply provides an interface for setting styles and rendering. Your platform specific implementation does the dirty work (using NSTextView in your example).

The benefit of the simple data class is that it is initially simple to write. Its drawback is that you will have a hard time avoiding a giant tangle of if-else statements. You will also be lacking a clear place to make you platform specific rendering calls that use the styles. As the system's complexity grows, you may find it more diffcult to decide where implementation details go.

The facade is a more abstracted approach. The benefits are that it is more flexible and can be reused if you decide to port to another platform. Its drawbacks are more upfront development time.

The facade's public interface will provide what you need to set and remove styles as well as initiate rendering when the time comes.

The details of how you want to set styles are up to you. Use what ever system feels best. Simple getters and setters or a generic set and get that uses a dictionary internally works too (see boosts ptree if you are using C++). You could even take all (or default) styles at construction time. You could decide to not even expose mutators at that point. Your call. Perhaps you decide it is important to data drive the styles you support and use a configuration plus factory system (We can add more detail later if that is important to you). In fact different implementatios of the facade could provide different ways of approaching the problem. You could prototype a few and choose what works best.

The platform specific implementation of you facade abstraction will use the platform specific rendering system (NSTextView in your case) and the styles you have set to make the appropriate calls to the system. Simply inject the platform specific classes at construction (Dependency Injection), implement your render() method and you should be good to go.

Part 3

If your system design allows, you could take all styles for a particular element at construction time. This could allow you to avoid getters and setters completely if you chose to make your element immutable. Your would then have a simple, clean and possibly immutable abstraction in front of your character styles system. Immutable state generally leads to fewer bugs but does require you to operate under the premise that you cannot change things willy-nilly.

Taking this a step further, a configuration file might define the different style setups you have. Again this would require prior knowledge of what styles you are setting (similar to constructing with the styles above). Give the type of style you are seeking, say "heading", you might fetch the configuration for headings which specifies a larger, bold font.

These are just some ideas off the top of my head. Without further requirements gathering and use cases it will be tough to get more specific.

Hope that helps. Good luck!

8
  • I've been trying to do that, and normally I don't have big problems coming up with a reasonable design. But for this class I just can't seem te figure it out... The functionality is clear to me: store style properties and return them, with inherintance and naming of styles. But how do you implement that? Like I said in my question, I feel the most logical solution would be to have a method format() and another called getFormatting(). But I can't do that. What would you do?
    – Frog
    Sep 9, 2012 at 16:36
  • What are you going to do with the properties once their and their parent styles are set?
    – hiwaylon
    Sep 9, 2012 at 19:12
  • Then I'm going to render the text on screen using a platform-dependant class. On Mac OS X for example, I will use NSTextView to render the text according to the styles. So basically all I need is a method to call which returns the requested style, liek getBold(). Do you understand?
    – Frog
    Sep 10, 2012 at 16:33
  • I do. This is why, in your other question thread, people were trying to give you what seemed like a text rendering engine. I've added another answer with a couple ideas for you.
    – hiwaylon
    Sep 10, 2012 at 20:38
  • The way you suggest I make the connection between the NSTextView and my C++ code is indeed the way I was planning to do it. But I don't really understand what you mean in the second to last paragraph: "You could even take all (or default) styles at construction time." "Perhaps you decide it is important to data drive the styles you support and use a configuration plus factory system" Could you please explain that? And by the, thanks for all the help thus far!
    – Frog
    Sep 12, 2012 at 13:07
2

I smell an attribution list. I agree you should set format and get format, except I would use a list. A bit mask even if your attributes allow. Otherwise just keep an array of formatter objects which use IOC to format there field if possible. forgive me, I don't know c++

Public abstract class IFormatter
{
.   public abstract void FormatText(TextField* fieldToFormat);
}

Public class BoldFormatter : IFormatter
{...}

Public class TextField
{
.   public void AddFormatter(IFormatter* formatterToAdd)
.   {...}

.   public IFormatter[] GetFormatters()
.   {...}

.   public void Render()
.   {
.       foreach(IFormatter formatter in formatters)
.       { formatter.FormatText(this); }
}
5
  • I find it a bit hard to understand what you're saying. Are you suggesting I create a class for every style property, and in the styles save a list of those styles, which I can look through? If so, wouldn't that be overkill?
    – Frog
    Sep 12, 2012 at 13:10
  • 1
    You would never need to "look through" that list, this is where the dependency inversion principle comes in: You have to have logic somewhere that knows how to do each individual type of formatting, why not have each individual formatter class do this, and then the text field will just have a list of formatters which you add to or remove from, and when it needs to render it tells all it's formatters to go to town on it. No need to pay attention to what formatters are there. en.wikipedia.org/wiki/Dependency_inversion_principle Sep 12, 2012 at 13:56
  • I understand it! And it's actually a really smart idea! But wouldn't it be overkill for such a simple situation?
    – Frog
    Sep 12, 2012 at 14:03
  • @Frog Maybe? It depends on how many formatters you have, and what you define as "overkill"? That's why I prefaced to mention the opposite end solution which is the simplest: a bit mask. Unfortunately with a bit mask you need the formatter logic for each individual formatter all bundled together somewhere which parses that bit mask (same if you used any other const form factor that didn't encapsulate the logic for formatting). If you only have 3 or 4, then all you need is a single byte and some router method that pulls it apart at render time to identify which are there. Less extensible though. Sep 12, 2012 at 14:59
  • Okay, I understand it. Thank you for the diverse answer!
    – Frog
    Sep 12, 2012 at 15:06
2

If all they do is store data, then why bother with any kind of getter and setter? You have no invariants to enforce, no logical reason to control the data, and no reason to process it nor prevent access. The class's reason to be is "Store data". So make it store some data and leave it at that.

2
  • True. I suggested this in the answer above. However an object with data usually does something with the data, by [Information Expert](en.wikipedia.org/wiki/GRASP_(object-oriented_design).
    – hiwaylon
    Sep 12, 2012 at 12:10
  • I can't do that, because the class does a bit more than just storing data. One thing is that the getters should not just return the requested style of the current object. If the requested style equals NULL (or some other initialization value), the getter should return the value of the parent style, which should in turn return the value of its parent style if its value equals NULL, etc. So basically I need to use methods.
    – Frog
    Sep 12, 2012 at 13:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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