Geek Answers Handbook

Is this design over engineering?

Could you use the interface and polymorphism to expand this design to over-engineering?

Pros

  • Extensible
  • encapsulated
  • Auto-magical

against

  • More code
  • A bit cumbersome to use (you have to use a different type name to get a different behavior)
  • May be less efficient to use due to virtual function calls.

My instinct is that for this particular case, one if and a boolean flag are an excellent option, but not everyone agreed with me.

What do you think?

Original

 // Connects to a local pipe, and naturally // owns that connection struct CommandWriter { CommandWriter() { fd = open("/path/to/fifo", O_WRONLY); if (fd == -1) throw std::runtime_error("Could not establish connection to FIFO"); }; ~CommandWriter() { close(fd); }; // (Has useful member functions here) private: CommandWriter(CommandWriter const&); // Not relevant to question int fd; };

Extended with boolean flag

 // Adds a constructor where an FD can be specified // from the call site, and no ownership is taken struct CommandWriter { CommandWriter() : owns_fd(true) { fd = open("/path/to/fifo", O_WRONLY); if (fd == -1) throw std::runtime_error("Could not establish connection to FIFO"); }; CommandWriter(int fd) : fd(fd), owns_fd(false) {}; ~CommandWriter() { if (owns_fd) close(fd); }; // (Has useful member functions here) private: CommandWriter(CommandWriter const&); // Not relevant to question int fd; bool owns_fd; };

Advanced with polymorphism

 // Sorry for the poor type names! struct ICommandWriter { virtual ICommandWriter() {} // (Has useful member functions here) private: ICommandWriter(ICommandWriter const&); // Not relevant to question }; struct CommandWriter_Connects : ICommandWriter { CommandWriter_Connects() { fd = open("/path/to/fifo", O_WRONLY); if (fd == -1) throw std::runtime_error("Could not establish connection to FIFO"); }; ~CommandWriter_Connects() { close(fd); }; // (Has useful member functions here) private: int fd; }; struct CommandWriter_Uses : ICommandWriter { CommandWriter_Uses(int fd) : fd(fd) {}; ~CommandWriter_Uses() {}; // (Has useful member functions here) private: int fd; };
+7
source share
6 answers

It depends on what you will use it for. If you have a large project and will use class options many times, then it certainly makes sense to make it flexible.

Rule of thumb:

  • Use it once - just keep it simple.
  • Use it twice - Keep it simple and make a copy and change as needed
  • Three or more - Summarize it and make it work in all cases.

Of course, there are many exceptions, but this is the starting point.

+9
source

Why would you just duplicate a file descriptor? That way, when an object is destroyed, you can just close () it and let the operating system take care of the rest:

 CommandWriter::CommandWriter (int _fd) : fd (dup (_fd)) {};

Adding a Boolean flag for this is to reinvent the wheel. And the use of polymorphism is the construction of a frigate helicopter.

+7
source

I agree with you that a logical solution is suitable here if you do not expect to add additional features in the future.

An alternative solution would be to use a Strategy Template . This is similar to custom removers for Boost smart pointers.

+1
source

I would have thought excessive engineering. The second code snippet is much shorter, and its use is simple. Using flags to indicate ownership of objects is not completely idiomatic, but it can be seen quite often, so I think most people will quickly understand the intention.

Keep it simple and stupid.

(I would prefer a polymorphic solution if it is sure that further code paths should be added in the future).

+1
source

I would prefer an interface. The reason is that the user of the interface understands that there may be various implementations. Perhaps in a month you will need to implement CommandWriter, which writes to db instead of a file (of course, you can even subclass the logical version, but this is not as obvious to the user as an interface).

Also for unit testing, I would say that an interface is a cleaner approach because you can implement a stub for classes that you want to test and that use ICommandWriter.

But, as mentioned above, if you are going to use it only once, just grab the Boolean version.

+1
source

In the polymorphism code:

 // (Has useful member functions here)
Nevertheless? If in all three places there are many member functions (a base class and two derived classes), then the owner and non-owner of the author are actually completely different animals. It is probably best to divide them into different classes, rather than into a class that behaves differently according to the logical flag set in it, according to which the constructor was called.

I suspect that all useful member functions are in the base class, and all derived classes are structural change and destruction. In this case, I need a smart_fd class that contains fd and knows how to dispose of it (you need two cases - call to close or do nothing. shared_ptr allows an arbitrary destruction function, but you probably don't need it here).

Then type one of them into CommandWriter and initialize it differently according to how the CommandWriter constructor is called.

Rule of thumb: classes that manage resources do nothing else.

+1
source

More articles:


All Articles