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.

Working with C++. Suppose I have a class BoxFilter. The class is used to filter boxes which have properties such as height, width, depth, weight, etc. The filter might have something like MaxWidth so that boxes with a width greater than MaxWidth would not pass the filter. Usage would be something like (C#-style pseudocode):

IBoxFilter filter = new BoxFilter();

foreach(Box box in boxes)
{
  if(filter.PassesFilter(box))
  {
     // do something  
  }
}

(sorry for the C# example but I think it's easier to understand)

In setting up the filter object I need to configure the settings of the filter. The filter currently has 7 properties although it could get more over time. I'm debating whether the filter class should have multiple setters such as SetWidthMax(), SetWidthMinimum(), SetHeightMax(), SetHeightMinimum(), etc or should I create a BoxFilterSettings object/struct and then have a single method on the box filter class called SetSettings(BoxFilterSettings settings)?

share|improve this question
    
I would have inverted your if statement and added a continue keyword to avoid possible braces by putting more than one line of code in the if block. –  The Muffin Man Sep 8 '11 at 1:01
2  
Avoiding braces is a bad idea. programmers.stackexchange.com/questions/16528/… –  Gary Buyn Sep 8 '11 at 1:16
    
Well as I said it's pseudocode. Would never avoid braces in real code. I'll fix it to avoid this line of discussion. –  User Sep 8 '11 at 2:38
    
Now there's something worth avoiding :) I was debating for a while whether I should bother making the comment. It was more for @Nick though. –  Gary Buyn Sep 8 '11 at 2:54
    
@Gary, avoiding braces looks cleaner to me. I've never, not even once had any code problems caused by writing in this style. It could happen though... –  The Muffin Man Sep 8 '11 at 3:46

7 Answers 7

up vote 5 down vote accepted

I personally prefer APIs with both. Sometimes I just want to set one criteria, so constructing a settings object is a pain. Other times, I want to reuse the same settings repeatedly, so setting them individually becomes a pain and I'll make my own settings wrapper object if one isn't provided.

share|improve this answer
    
Ultimately what I realized is you're right if I want to set just one setting it will be a pain having to set a settings object. –  User Sep 10 '11 at 22:23

I would turn the problem on its head.

Some people want to filter on width, others on height, others want only square forms, or rectangles that roughly have a 16:9 aspect...

You cannot possibly propose all alternatives.

Therefore the easier is to propose an interface:

class IFilter: boost::noncopyable {
public:
  virtual bool passes(Box const& box) const = 0; // const or not ?
  virtual ~IFilter() {}
};

And perhaps setup some easy filters:

class MaxWidthFilter: public IFilter {
public:
  MaxWidthFilter(size_t max, IFilter const* next = 0): _max(max), _next(next) {}

  virtual bool passes(Box const& box) const {
    if (box.getWidth() > _max) { return false; }
    return _next ? _next->passes(box) : true;
  }

private:
  size_t _max;
  IFilter const* _next;
};

Note: you might want to go full cloning/owned here.

Advantages:

  • Extensible
  • Order of tests is configurable
  • Can do more than filtering (ie: statistics on tests, ...)

Disadvantages:

  • A bit more complicated to begin with (though you already have 7 properties...)
  • A bit more slower probably, though the possibility to reorder tests depending on the collection to filter might outweight this

Note: you could recognize a Decorator here, or perhaps a Chain of Responsability.

share|improve this answer
    
You bring up a really good point. You know I had thought about something more extensible that would allow me to chain together filters. Seems similar to the builder pattern but different. Almost like building a chain of signal effects processing. With your example, is the idea that you would call MaxWidthFilter(10, MaxHeightFilter(20)).passes(box)? –  User Sep 8 '11 at 18:53
    
@User: among other things, this chaining allows for a great degree of flexibility. You can even create OrFilter(MaxWidthFilter(10), MaxHeightFilter(20)).passes(box) :) –  Matthieu M. Sep 8 '11 at 19:06
    
Interesting. I'm wondering what the best way to implement that is. I suppose you could also make a "parent" filter an ordered collection of filters and iterate over the filter collection and only if everything passes does the "parent" filter pass (although then you couldn't do your orfilter example) –  User Sep 8 '11 at 19:26
    
It sounds like you're proposing Composite pattern –  Kevin Sep 8 '11 at 20:26
    
@User: I would stick to the Decorator/Chain of Responsability approach, they are more flexible. –  Matthieu M. Sep 9 '11 at 6:05

If you're using C++ and you're expecting the options to be known when you create the object (i.e. you're not just defaulting them to a specific value and then modifying them based on user input), then common practice is to pass in the data via either multiple parameters or a single init object in the class constructor so that you can take advantage of the initialization list.

struct BoxParams
{
    int widthMax;
    int widthMin;
};

class BoxFilter(BoxParams* params)
: mWidthMax(params->widthMax)
, mWidthMin(params->widthMin)
{
}

Or something along those lines..

Personally, I prefer using initialization objects as the API is almost guaranteed to not change (well, at least for the initialization stuff), so you don't have to worry about issues such as parameter order when updating your initialization struct.

share|improve this answer
    
"you're not just defaulting them to a specific value and then modifying them based on user input" actually that's exactly what I'm doing ;) But more importantly, the settings need to be changeable after object construction. –  User Sep 8 '11 at 1:09
    
@User: Then you can always add an Update method to the BoxFilter class, using the same object as passed into the class c'tor. –  Demian Brecht Sep 8 '11 at 1:10
    
+1 for mentioning the benefit of the API not needing to change; this is a huge consideration as projects, groups, components, etc. start to get larger & larger. –  Radian Sep 8 '11 at 3:01
    
What the hell BoxParams* params !! Come on boy, you shall do better than that! (Hint: const, Hint 2: nullity/reference). –  Matthieu M. Sep 8 '11 at 18:24
    
@Matthieu: I did say "or something along those lines" ;). I didn't really care about constness or null checking. I was going to change the pointer to reference, but had already submitted and thought "meh, if he understands C++, then should know which to use and why". The code was intended more for the sake of using initialization lists and nothing more. While we're on nitpicky details, the ints in the struct should also probably be unsigned 16 bit as well as you shouldn't have negative widths and exceeding 65535 would probably be overkill unless you're dealing with twips. –  Demian Brecht Sep 8 '11 at 18:37

The filter currently has 7 properties although it could get more over time. I'm debating whether the filter class should have multiple setters {snip} or should I create a BoxFilterSettings object/struct

I think you will end up doing both. Based on experiences (at my previous employer) with some applications that have been sold for over 15 years, you will slowly add more and more properties. This is normal. But a constructor with 20 parameters is not. Nor is having 20 different constructors, all which vary by 1 setting.

What we ended up doing:

  • We left the setters/getters. Adding more as the years go by.

  • We created a big settings object. This settings object would be passed to the constructor (the number of constructors was reduced to 2: one with settings, one with none). We also had a separate settings setter that took this settings object.

share|improve this answer

There is very big problems with complexity in the example above:

  1. The actual structure of the problem is 2d array (of booleans) where one dimension is boxes and another dimension is the properties. This structure is pretty much coming from visual basic from 1980.
  2. Then there is arbitrary boolean expressions manipulating these booleans from the 2d array

These two features together make it extreamly complex. For example, testing that the code works requires going through all elements of the 2d array (and evaluating boolean expression in each element of the 2d array) and then further evaluating the boolean expressions that connect the rows and columns of the 2d array. It's like adding your excel spreadsheet to your program and then making it calculate some values from the data; except that you need to add the setters/getters manually and manipulating the data is very burdensome. The complexity is still there.

So my recommendation is to try to make it simpler somehow - reducing rows/columns is a start, but try to get rid of the 2d array structure. So that you don't need to implement whole excel for it. Excel also uses this kind of 2d array where each node can have differently typed data in it. These conventions coming from C# seems to have this kind of hidden complexity in them.

Here's some c++ code which is pretty much equivalent of your code above, and allows you to see the actual 2d array structure:

template<class T> class Array1d { virtual T Map(int x) const=0; };
class Array2d { virtual bool Map(int x, int y) const=0; };
class Array2dImpl : public Array2d { 
     Array2dImpl(Array1d<Box> &b) : b(b) { } 
     bool Map(int x, int y) const 
        { if (y==0) { return b.Map(x).width < maxwidth; }
           if (y==1) { return b.Map(x).height < maxheight; }
         ...  }
   Array1d<Box> &b;
   int maxwidth;
   int maxheight;
 };
 class BooleanExpression : public Array1d<bool>
 {
 public:
     BooleanExpression(Array2d &a) : a(a) { }
     bool Map(int x) const { return a.Map(x,0) && a.Map(x,1) && a.Map(x,2) && a.Map(x,3); }
 private:
     Array2d &a;
 };

Like your example above, this code can be used with a simple piece of code:

int main() {
   Array1d<Box> boxes;
   Array2dImpl array(boxes);
   BooleanExpression e(array);
   for(int i=0;i<e.size();i++) 
      {
       if (e.Map(i)) { /* do something */ }
      }
  }

Now returning to your question about how to set the maxwidth variable in this example. It might seem that it requires separate setter for each property. But this is not the case, actually, best way forward is to add a function like this:

void SetElement(int x, int y, bool b);

This naturally belongs to the 2d array which is already included in your example. The real problem with this example is that there is 2^(x*y) different states the system can be in. This comes from x*y calls to SetElement. This kind of state explosion is completely crazy for such a system and this will break your software very badly.

Even if we assume you want to generate x*y booleans using the code above by just providing the dimensions, the trick how your example did the evil complexity is very hidden. Passing the box from foreach to PassesFilter() function looks very simple, but it causes all this trouble, reconfiguring all the booleans in single line of the excel. Related is also the state space involved in the coordinates of the boxes. Those are even worse problem.

(and no, hiding the 2d array by using C# code does not make it easier to understand,)

(In fact, it's very evil to post such code for other programmers to read, I hope you didn't know the 2d array is there and you still have a chance to somehow fix your software...)

share|improve this answer
    
I'm not sure if I understand what you're getting at. Yes I could view a collection of objects and their associated properties as a 2 dimensional array. But part of the whole point of object oriented programming is to reduce complexity, and you seem to want to expose the complexity effectively removing the abstractive power of object oriented programming. I don't want to think in terms of 2D arrays. I want to think of collection of boxes. Also the C++ version of the code would be nearly identical, just using an iterator. But what do you propose would be a better solution? –  User Sep 8 '11 at 22:02
    
No, the problem with abstraction is that it does not remove the complexity. It only hides it. So your software still is very buggy, you've just made it magically disappear. The better solution would be to make it simple instead. And first step is to understand where the complex stuff is coming from, and which part is actually necessary. 2^(x*y) complexity is probably not necessary. –  tp1 Sep 8 '11 at 22:44
1  
You sir are either a genius or a madman. I will either up or downvote once I figure which way it is. –  Kevin Sep 9 '11 at 4:52
    
But I'm pretty sure complex != buggy –  Kevin Sep 9 '11 at 4:53
    
complex just makes it more difficult to keep it free of errors. –  tp1 Sep 9 '11 at 15:24

My experience with jQuery tells me that passing a settings object is much more easier than having many setter methods. However, jQuery can do that, because it supports object literal notation. In other words, you can very simply create an object on the fly with minimum code and pass it to a function.

C# couldn't do that till it introduced the concept o object initializer alongside the concept of anonymous types.

I don't know about C++ syntax. But if you should create a strongly typed object first, and declare parameters for each setting, then create an instance of that class, and then assign values to each property on a new line, to create a settings object, I think sticking with the single method would be more fine.

share|improve this answer

Two words: Builder Pattern. And also, you're welcome.

For example:

class BoxFilter {
    friend BoxFilterBuilder;
    private int height;
    private int width;
};
class BoxFilterBuilder {
    int height_;
    int width_;
    public:
    BoxFilterBuilder& height(int x) {this.height_ = x; return *this;}
    BoxFilterBuilder& width(int x) {this.width_ = x; return *this;}
    BoxFilter build() {
        BoxFilter filter;
        filter.height = height_;
        filter.width = width_;
        return filter;
    }
};

Usage is something like this:

BoxFilter filter = BoxFilterBuilder().height(12).width(1).build();

As you can see the syntax is clear and remains so regardless the number of construction parameters. And the box filter is always constructed in a correct state (you can guarantee that with some validations in your build() method. Further you can force creation via the builder by making the constructor of BoxFilter private.

share|improve this answer
    
I felt the need to provide a builder example because of the inexplicable downvotes. –  Kevin Sep 8 '11 at 15:37
    
I was going to suggest builder pattern too... can't imagine why people have downvoted this :s –  MattDavey Sep 8 '11 at 15:55
    
I didn't downvote you but I think your original answer could have used more fleshing out (compare to this answer on SO: stackoverflow.com/questions/328496/…). Secondly I don't think it's the right answer because I think it's trying to solve a problem I don't really have: avoiding telescoping constructor parameters, guaranteeing object state after construction and/or having predefined filters. Overall I think for my use-case it adds unneeded complexity, although I'm open to exploring it further. –  User Sep 8 '11 at 17:55
    
My original answer assumed people knew what builder is or were at least willing to look it up on the web. Maybe that assumption was bad –  Kevin Sep 8 '11 at 20:06

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.