CODEWITHSUNDEEP
c++oopdesign-patternswrapper
Mukul Gupta
Mukul Gupta

Reputation: 2425

Design a good C++ wrapper class that wraps around multiple functionalities

I want to design a wrapper class in C++ that implements File transfer using FTP, SFTP.

I have a base class FileTransfer (using curl) that is inherited by derived class FTP. I need to support SFTP, so I have implemented another derived class SFTP that also inherits from FileTransfer.

I am making a wrapper class code along the following lines. However, this doesn't look like good design. I am relatively new to OOP, although I have worked on C in the past.

class Wrapper {
  public:
  Wrapper(int m_protocol){
    protocol = m_protocol;
    if (protocol)
      pftp = new FTP();
    else
      psftp = new SFTP();
  }
  
  ~Wrapper() {
    if (protocol)
      delete pftp;
    else
      delete psftp;
  }
  //Function supported by both ftp/sftp
  void do_something(){
    if (protocol)
      pftp->do_something();
    else 
      psftp->do_something();
  }
  
  //FTP specific function
  void use_passive(){
    assert(protocol);
    pftp->use_passive();
  }
  
  //SFTP specific function
  void ssh_key(){
    assert(!protocol);
    psftp->ssh_key();
  }
  
  private:
   int protocol;
   FTP *pftp;
   SFTP *psftp;
};

How can I improve this design? How can I avoid the if (protocol) check inside every function and improve the code quality? Should I instead use void pointers for psftp and 'pftp`?

Edit: I am using a wrapper because at a lot of places in the project, existing FTP object is being used and if I use a separate class for SFTP (without a wrapper), I will have to add an if check every time to also support SFTP. I don't want to expose the detail (FTP/SFTP) to the caller.

Upvotes: 2

Views: 1485

Answers (2)

Christophe
Christophe

Reputation: 73406

All you need is polymorphism using a pointer to FileTransfer, making do_something() and the ~FileTransfer() destructor virtual functions (i.e. you invoque the function on the base object pointer, and the object will call the right function depending on its real class).

The remaining question is only about the construction of your object based on the protocol. The correct term would not be "wrapper" but "factory" (design pattern). This could be realized by a static member function of the FileTransfer.

Upvotes: 2

Barry
Barry

Reputation: 303147

Everything here can be accomplished easier by just using your base class pointer.

FileTransfer* ft;
std::unique_ptr<FileTransfer> ft; // C++11

Making one:

// should this really be an int?
FileTransfer* factory(int protocol) {
    if (protocol)
        return new FTP;
    else
        return new SFTP;
}

// in C++11 this should be
std::unique_ptr<FileTransfer> factory(int protocol);

Doing something:

ft->do_something();

Doing something specific to one or the other:

// this will die if ft is an SFTP
dynamic_cast<FTP*>(ft)->use_passive();

// but you could check it
if (FTP* ftp = dynamic_cast<FTP*>(ft)) {
    ftp->use_passive();
}

// or perhaps even better, make a default virtual that does nothing
virtual void FileTransfer::use_passive() { }

void FTP::use_passive() override { // whatever }

ft->use_passive();

Deleting:

// make sure FileTransfer::~FileTransfer() is virtual!!
delete ft;

Upvotes: 2

Related Questions