76

I am currently working on a software project that performs compression and indexing on video surveillance footage. The compression works by splitting background and foreground objects, then saving the background as a static image, and the foreground as a sprite.

Recently, I have embarked on reviewing some of the classes that I have designed for the project.

I noticed that there are many classes that only have a single public method. Some of these classes are:

  • VideoCompressor (with a compress method that takes in an input video of type RawVideo and returns an output video of type CompressedVideo).
  • VideoSplitter (with a split method that takes in an input video of type RawVideo and returns a vector of 2 output videos, each of type RawVideo).
  • VideoIndexer (with an index method that takes in an input video of type RawVideo and returns a video index of type VideoIndex).

I find myself instantiating each class just to make calls like VideoCompressor.compress(...), VideoSplitter.split(...), VideoIndexer.index(...).

On the surface, I do think the class names are sufficiently descriptive of their intended function, and they are actually nouns. Correspondingly, their methods are also verbs.

Is this actually a problem?

Doc Brown
  • 218,378
Yong Jie Wong
  • 871

9 Answers9

106

No, this is not a problem, quite the opposite. It is a sign of modularity and clear responsibility of the class. The lean interface is easy to grasp from the viewpoint of a user of that class, and it will encourage loose coupling. This has many advantages but almost no drawbacks. I wish more components would be designed that way!

Doc Brown
  • 218,378
30

It is no longer object oriented. Because those classes don't represent anything, they are just vessels for the functions.

That does not mean it's wrong. If the functionality is sufficiently complex or when it's generic (i.e. the arguments are interfaces, not concrete final types), it makes sense to put that functionality in separate module.

From there it depends on your language. If the language has free functions, they should be modules exporting functions. Why pretend it's a class when it isn't. If the language does not have free functions like e.g. Java, then you create classes with single public method. Well, that just shows the limits of object oriented design. Sometimes functional is simply better match.

There is one case when you may need a class with single public method because it has to implement interface with single public method. Be it for observer pattern or dependency injection or whatever. Here it again depends on the language. In languages that have first class functors (C++ (std::function or template parameter), C# (delegate), Python, Perl, Ruby (proc), Lisp, Haskell, ...) these patterns use function types and don't need classes. Java does not (yet, will in version 8) have function types, so you use single method interfaces and corresponding single method classes.

Of course I am not advocating writing single huge function. It should have private subroutines, but they can be private to the implementation file (file-level static or anonymous namespace in C++) or in a private helper class that is only instantiated inside the public function (To store data or not?).

Community
  • 1
Jan Hudec
  • 18,410
14

There may be reasons to extract a given method into a dedicated class. One of those reasons is to allow Dependency Injection.

Imagine you have a class called VideoExporter which, eventually, should be able to compress a video. A clean way would be to have an interface:

interface IVideoCompressor
{
    Stream compress(Video video);
}

which would be implemented like this:

class MpegVideoCompressor : IVideoCompressor
{
    // ...
}

class FlashVideoCompressor : IVideoCompressor
{
    // ...
}

and used like this:

class VideoExporter
{
    // ...
    void export(Destination destination, IVideoCompressor compressor)
    {
        // ...
        destination = compressor(this.source);
        // ...
    }
    // ...
}

A bad alternative would be to have a VideoExporter which has plenty of public methods and does all the job, including the compressing. It would quickly become a maintenance nightmare, making it hard to add support for other video formats.

Arseni Mourzenko
  • 137,583
9

This is a sign that you want to pass functions as arguments to other functions. I'm guessing your language (Java?) doesn't support it; if that's the case, it's not so much a failing in your design as it is a shortcoming in your language of choice. This is one of the biggest problems with languages that insist that everything must be a class.

If you aren't actually passing these faux-functions around then you just want a free/static function.

Doval
  • 15,487
9

I know I'm late to the party but as every one seems to have missed to point this out:

This is a well known design pattern called: Strategy Pattern.

Strategy pattern is used when there are several possible strategies to solve a sub-problem. Typically you define an interface that enforces a contract on all implementations and then use some form of Dependency Injection to provide the concrete strategy for you.

For example in this case you could have interface VideoCompressor and then have several alternative implementations for example class H264Compressor implements VideoCompressor and class XVidCompressor implements VideoCompressor. It is not clear from OP that there is an interface involved, even if there is not, it may simply be that the original author left the door open to implement strategy pattern if needed. Which in and of itself is good design too.

The problem that OP constantly finds herself instantiating the classes to call a method is a problem with her not using dependency injection and the strategy pattern correctly. Instead of instantiating it where you need it, the containing class should have a member with the strategy object. And this member should be injected, for example in the constructor.

In many cases the strategy pattern results in interface classes (as you are showing) with just a single doStuff(...) method.

Emily L.
  • 362
2

It is a problem - you are working from the functional aspect of the design, rather than the data. What you actually have are 3 standalone functions that have been OO-ified.

For example, you have a VideoCompressor class. Why are you working with a class designed to compress video - why do you not have a Video class with methods on it to compress the (video) data that each object of this type contains?

When designing OO systems, its best to create classes that represent objects, rather than classes that represent activities that you can apply. In the old days, classes were called types - OO was a way to extend a language with support for new data types. If you think of OO like this, you get a better way of designing your classes.

EDIT:

let me try to explain myself a little better, imagine a string class that has a concat method. You can implement such a thing where each object instantiated from the class contains the string data, so you can say 

string mystring("Hello"); 
mystring.concat("World");

but the OP wants it to work like this:

string mystring();
string result = mystring.concat("Hello", "World");

now there are places where a class can be used to hold a collection of related functions, but that is not OO, its a handy way of using the OO features of a language to help manage your codebase better, but it is no way any kind of "OO Design". The object in such cases is totally artificial, simply used like this because the language does not offer anything better to manage this kind of problem. eg. In languages such as C# you would use a static class to provide this functionality - it reuses the class mechanism, but you no longer need to instantiate a object just to call the methods on it. You do end up with methods like string.IsNullOrEmpty(mystring) which I think is poor compared to mystring.isNullOrEmpty().

So, if anyone is asking "how do I design my classes", I recommend thinking of the data the class will contain rather than the functions it contains. If you go for the "a class is a bunch of methods", then you end up writing "better C" style code. (which isn't necessarily a bad thing if you are improving C code) but it is not going to give you the best OO designed program.

gbjbaanb
  • 48,749
  • 7
  • 106
  • 173
2
public interface IVideoProcessor
{
   void Split();

   void Compress();

   void Index();
}

What you've got is modular and that's good, but if you were to group these responsibilities into IVideoProcessor, that would probably make more sense from DDD point of view.

On the other hand, if splitting, compressing and indexing wasn't related in any way, than I'd keep them as separate components.

CodeART
  • 4,060
  • 1
  • 22
  • 23
0

The ISP (interface segregation principle) says that no client should be forced to depend on methods it does not use. The benefits are multiple and clear. Your approach totally respects the ISP, and that's good.

A different approach also respecting the ISP is, for example, create an interface per each method (or set of methods with a high cohesion) and then have a single class implementing all those interfaces. Whether this is or not a better solution depends on the scenario. The benefit of this is, when using dependency injection, you could have a client with diferent collaborators (one per each interface) but in the end all the collaborators will point to the same object instance.

By the way, you said

I find myself instantiating each class

, these classes seem to be services (and thus stateless). Have you thought about making them singletons?

diegomtassis
  • 195
  • 1
  • 7
0

Yes, there is a problem. But not severe. I mean you can structure your code like this and nothing bad will happen, it is maintainable. But there are some weaknesses to that kind of structuring. For example consider if representation of your video (in your case it is grouped in RawVideo class) changes, you will need to update all your operation classes. Or consider that you might have multiple representations of video that vary in runtime. Then you will have to match representation to particular "operation" class. Also subjectively it is annoying to drag around a dependency for each operation you want to do on smth. and to update list of dependencies passed around every time you decide that operation is no longer needed or you need new operation.

Also that's actually a violation of SRP. Some people just treat SRP as a guide for splitting responsibilities (and take it to far by treating each operation a distinct responsibility) but they forget that SRP is also a guide to grouping responsibilities. And according to SRP responsibilities that change for same reason should be grouped together so that if change happens it is localized to as little classes/modules as possible. As for big classes it is not a problem having multiple algorithms in same class as long as those algorithms are related (i.e. share some knowledge that should not be known outside of that class). The problem are big classes that have algorithms that are not related in any way and change/vary for different reasons.

ddiukariev
  • 19