CODEWITHSUNDEEP
c++design-patternsinterface
Patrick
Patrick

Reputation: 23619

Having one interface with lots of virtual methods? Or having many interfaces with only 1 virtual method?

I have a C++ module that needs to get information from other classes, without knowing those classes. The obvious approach is to use interfaces.

Let me give you an example. Suppose I have a library that manages books, and all books have their own characteristics and functionalities, and to allow the library to get a characteristic from a book or execute a functionaity, the book needs to implement an interface. Like this:

class Library
   {
   public:
      void addBook(IBook &book);
   };

class IBook
   {
   public:
      string getAuthor()    = 0;
      string getTitle()     = 0;
      string getISBNCode()  = 0;
      size_t getNofPages()  = 0;
      size_t getNofImages() = 0;
      double getPrice()     = 0;
      void   printBook()    = 0;
      void   convertToPdf() = 0;
   };

Unfortunately, it does not make sense to implement all these methods for all kinds of books.

Because I only have 1 interface, I am forced to implement everything for all books and return 0, return "" or do nothing int he implementation if it is irrelevant.

An alternative could be to split these interfaces in many interfaces, like this:

class IBook
   {
   public:
      string getAuthor()    = 0;
      string getTitle()     = 0;
      size_t getNofPages()  = 0;
   };

class IISBNGetter
   {
   public:
      string getISBNCode()  = 0;
   };

class IImagesGetter
   {
   public:
      size_t getNofImages() = 0;
   };

class IBuyable
   {
   public:
      double getPrice()     = 0;
   };

class IPrintable
   {
   public:
      void   printBook()    = 0;
   };

class IConvertible
   {
   public:
      void   convertToPdf() = 0;
   };

Book classes then only need to implement the interfaces they really want to support.

Adding a book to the library then becomes something like this:

bookid = myLibrary->addBook (myBook);
myLibrary->setISBNGetter  (bookid, myBook);
myLibrary->setImageGetter (bookid, myBook);
myLibrary->setBuyable     (bookid, myBook);

The advantage of having different interfaces is that it is clear for the library who supports what, and it never has the risk of calling something that is simply not supported.

However, since every book can have any possible combination of characteristics/functionalities, I end up with lots of interfaces with only 1 method.

Isn't there a better way to organize the interfaces to obtain something like this?

I was also thinking about using Lambda expressions but behind the screens this is almost the same as having many many interfaces with only 1 method.

Any ideas?

Upvotes: 5

Views: 660

Answers (7)

Steve Jessop
Steve Jessop

Reputation: 279245

I think you should draw a distinction between actually having an ISBN, and implementing an interface that queries ISBN (in this case, IBook).

There's no reason you can't say, as a part of the definition of "book", that a book is something of which "it is possible to find whether it has an ISBN, and if so what".

If you don't like returning empty values to indicate "none", that's fair enough. In some domains, an empty string is a valid value, so that wouldn't even be possible. You could have:

bool hasISBNcode();
string getISBNcode();

or:

std::pair<bool, string> getISBNcode();

or similar.

Otherwise, you'll find yourself with dynamic_cast all over the place before you call any of the functions of IBook, and you'll also find yourself with 2^n different concrete classes for different types of books. Or, in your example code, you involve the library in the business of whether a book has an ISBN (which seems wrong to me - it's nothing to do with the library, it's a property of the book alone). None of those is particularly fun to work with, and they don't seem necessary here.

If those kinds of things looked necessary, you could perhaps use strategies. Define ConcreteBook as able to look for an ISBN using some helper object, without the book class knowing how the search is performed. Then plug in different objects to do that looking, depending whether a particular book actually has one or not. Seems a bit overkill, though, for what is probably just a nullable column in a database somewhere.

Upvotes: 2

Cheers and hth. - Alf
Cheers and hth. - Alf

Reputation: 145239

There are two just slightly (ever so slightly!) related problems:

  • Proper abstraction of logical parts hierarchy.
  • The possibility of null-values.

Others have provided solutions to each problem. Namely, regarding the first, don't go for an everything interface, don't go for single-method-per-interface, but try to model the hierarchy that's there. And regarding the latter, boost::optional, possibly augmented with separate query methods for existence of data item.

I just want to emphasize, which may not be apparent from the answers present as I'm writing this, that they really are two separate problems.

Regarding style (another aspect of clarity), what's all this getSin Javaism stuff?

x = 2*getSin(v)/computeCos(v)

Doesn't make sense in C++, just write sin. :-)

Cheers & hth.,

Upvotes: 1

Dialecticus
Dialecticus

Reputation: 16761

You could have for each book a container of pointers to base-interface singleton objects, something like std::map<std::string, IBase>. Then you would require the interface by name, get the pointer to it (or null), and just call some doDefault() on it (or upcast the pointer to IDerived, if must). Each interface function would have to have pointer to Book as its first (or even only) parameter: doDefault(const Book*).

Upvotes: 1

BЈовић
BЈовић

Reputation: 64223

Another solution would be to keep the interface, but to use boost::optional for return values that can be empty.

class IBook
   {
   public:
      virtual ~Ibook(){}

      virtual string getAuthor()    = 0;
      virtual string getTitle()     = 0;
      virtual string getISBNCode()  = 0;
      virtual size_t getNofPages()  = 0;
      virtual size_t getNofImages() = 0;
      virtual boost::optional< double > getPrice()     = 0;   // some have no price
      virtual void   printBook()    = 0;
      virtual void   convertToPdf() = 0;
   };

Upvotes: 1

Arseny
Arseny

Reputation: 7351

I guess you don't need to get to any extremes but to choose middle way. Having one interface is not good it breaks Interface Segregation Principle (ISP) on other hand to having so many interfaces will spoil your code as well. I would leave one core IBook and think over the rest. For instance IPrintable and IConvertible(for pdf convert) may go in one interface. Probably IBuyable and IISBNGetter will alse go togehter.

Upvotes: 2

SirDarius
SirDarius

Reputation: 42879

A solution could be to keep your base interface with pure virtual methods, but have your actual implementations inherit from an intermediate class providing default implementations for the virtual methods.

One common implementation of that intermediate class would be to throw some kind of "MethodNotImplemented" exception in each method, so the user of the class can catch those on a case-by-case basis.

Seeing as exceptions are a bit expensive for such a case where calling non-existing methods would not be "exceptional", there is also the approach of having those methods being empty or returning default values, as Simone posted.

Upvotes: 2

Simone
Simone

Reputation: 11797

I'd have IBook to implement every method:

class IBook
   {
   public:
      virtual ~IBook() {}

      virtual string getAuthor() { return ""; } // or some other meaningful default value
      virtual string getTitle() { return ""; }
      virtual string getISBNCode() { return ""; }
      virtual size_t getNofPages() { return 0; }
      virtual size_t getNofImages() { return 0; }
      virtual double getPrice() { return .0; }
      virtual void   printBook() {}
      virtual void   convertToPdf() {}
   };

since your alternative is too messy for me, you'd end with lots of confusing interfaces. Otherways, you could check if the Visitor pattern could be applied here.

Upvotes: 8

Related Questions