2
\$\begingroup\$

I'm learning command design pattern right, so I decided to make this project. Basically it's a remote controller and it issues commands to VCR. I've seen different implementations of this pattern so I'm not really sure if I got it right. So the key points are:

  • Is the implementation correct? If not, what's the problem?
  • From what I understand VCRRemote class is both invoker and client. Does it break the pattern?
  • I wasn't really sure where to put key parser functions so I made a namespace. Should I put them inside the VCRRemote or maybe create another class (inside VCRRemote)?
  • Are raw pointers the way to go here or did I miss something? When I'm working with pointers should I wrap them in try-catch block or check for nullptr like I did?
  • Is there a better way to parse input in console application? Maybe some libraries?
  • Overall code style.

BTW I know that I shouldn't use conio.h and aim for the cross-platform but I decided not to over complicate things.

ACommand.h

#ifndef ACOMMAND_H
#define ACOMMAND_H

#include "VCR.h"

class ACommand {
public:
 ACommand(VCR* vcr) : vcr_(vcr) {};
 virtual ~ACommand() {};
 virtual void execute() = 0;
protected:
 VCR* vcr_;
};

#endif 

Commands.h

#ifndef COMMANDS_H
#define COMMANDS_H

#include "ACommand.h"
#include "VCR.h"

class PowerCommand : public ACommand {
public:
 PowerCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->toggle_power(); 
 }
};
class PlayCommand: public ACommand {
public:
 PlayCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->play(); 
 }
};
class PauseCommand : public ACommand {
public:
 PauseCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->pause();
 }
};
class StopCommand : public ACommand {
public:
 StopCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->stop(); 
 }
};
class RecordCommand : public ACommand {
public:
 RecordCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->record(); 
 }
};
class FFCommand : public ACommand {
public:
 FFCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->fast_forward(); 
 }
};
class RewindCommand : public ACommand {
public:
 RewindCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->rewind(); 
 }
};
class InsertCommand : public ACommand {
public:
 InsertCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->insert(nullptr); 
 } 
};
class EjectCommand : public ACommand {
public:
 EjectCommand(VCR* vcr) : ACommand(vcr) {};
 inline void execute() { 
  if (vcr_) vcr_->eject(); 
 }
};

#endif

VCRRemote.h

#ifndef VCRREMOTE_H
#define VCRREMOTE_H

#include <memory>

#include "VCR.h"
#include "ACommand.h"

class VCRRemote;

namespace parser {
 void read_key(VCRRemote&);
 void parse_command(VCRRemote&, int);
}

class VCRRemote {
public:
 VCRRemote(const VCR& vcr) : command_(nullptr), vcr_(vcr) {};

 friend void parser::parse_command(VCRRemote&, int);
 inline void execute() { 
  if (command_) command_->execute(); 
 }
private:
 std::unique_ptr<ACommand> command_;
 VCR vcr_;
};

#endif

VCRRemote.cpp

#include <conio.h>

#include "VCRRemote.h"
#include "Commands.h"

void parser::read_key(VCRRemote& remote) {
 int key = 0;
 while (key != 'y') {
  key = _getch();
  parse_command(remote, key);
  remote.execute();
 }
}
void parser::parse_command(VCRRemote& remote, int key) {
 switch (key) {
 case 't':
  remote.command_.reset(new PowerCommand(&remote.vcr_));
  break;
 case 'q':
  remote.command_.reset(new PlayCommand(&remote.vcr_));
  break;
 case 'w':
  remote.command_.reset(new PauseCommand(&remote.vcr_));
  break;
 case 'e':
  remote.command_.reset(new StopCommand(&remote.vcr_));
  break;
 case 'r':
  remote.command_.reset(new RecordCommand(&remote.vcr_));
  break;
 case 'a':
  remote.command_.reset(new FFCommand(&remote.vcr_));
  break;
 case 's':
  remote.command_.reset(new RewindCommand(&remote.vcr_));
  break;
 case 'd':
  remote.command_.reset(new InsertCommand(&remote.vcr_));
  break;
 case 'f':
  remote.command_.reset(new EjectCommand(&remote.vcr_));
  break;
 default:
  remote.command_.reset(nullptr);
  break;
 }
}

main.cpp

#include "VCR.h"
#include "VCRRemote.h"

int main() {
{
 VCR vcr;
 VCRRemote remote(vcr);
 parser::read_key(remote);
 return 0;
}
\$\endgroup\$
1
\$\begingroup\$

Design Patterns

The things to grasp about design patters is that they are not techniques on how to implement something. They are basically a way no name things not a pattern on how to build them.

Basically they allow computer programers to talk about a design without having to delve into implementation details. See Are design patterns really essential nowadays?

I've seen different implementations of this pattern so I'm not really sure if I got it right.

Yes that's totally fine. The design pattern gives you an overview of the technique. The actual implementation will depend entirely on the problem space. You should not be forcing yourself to change your code to fit the pattern. You usually write the code then say hey I used "Pattern XYZ". Now knowing the patterns will give you an opportunity to see techniques that you may not have come across with your current experience, so studying them is useful.

Is the implementation correct? If not, what's the problem?

Wrong question. Its not a matter of correctness. Does the program work. Does it solve the problem. These are the correct questions.

Does it look approximately like the pattern you think you are using is fine if you want to describe your application to another developer.

The command Pattern

Yes your design implements the command pattern. But I would have done it slightly differently. I may have done it slightly differently (but that's the point).

Are raw pointers the way to go here or did I miss something?

Raw pointers are usually not the answer. But sometimes they are necessary. Personally I would use a reference nearly everywhere you are using a pointer (assuming the pointer could never be nullptr).

When I'm working with pointers should I wrap them in try-catch block or check for nullptr like I did?

Check them for NULL before use always. But that is why I use references. Check of the object exists once (not on every usage). You can create a NULL object to handle the situations for nullptr.

Is there a better way to parse input in console application? Maybe some libraries?

Std::cin is the best way to get console input.

Review od Code

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.