Adapter pattern in C++ returning a segmentation fault

Question

I am trying to learn C++ and design patterns, I am doing this by translating the Java code from this page to C++ and run it so I can learn advanced C++ syntax (at least I think so, correct me if I am wrong). I translated to code in that page, and compiled it using Digital Mars. It compiled, but when I try to run it, I get nothing. I went online and tried several online C++ compilers, it compiles but running it returns a segmentation fault errors which is to the best to my knowledge is trying to access a memory location that I am not allowed to.

I am new to C++, what did I do wrong to cause such an error.

#include <iostream>
#include <string>

using namespace std;

class MediaPlayer {
public:
 virtual void play(string audioType, string fileName);
};

class AdvancedMediaPlayer { 
public:
 virtual void playVlc(string fileName);
 virtual void playMp4(string fileName);
};

class VlcPlayer : public AdvancedMediaPlayer{
    
public:
void playVlc(string fileName) {
      cout << "Playing vlc file. Name: "<< fileName;        
   }

    
void playMp4(string fileName);
};

class Mp4Player : public AdvancedMediaPlayer{
    
public:
void playVlc(string fileName);
    
void playMp4(string fileName) {
      cout << "Playing mp4 file. Name: "<< fileName;        
   }
};

class MediaAdapter : public MediaPlayer {

AdvancedMediaPlayer* advancedMusicPlayer;

public:
MediaAdapter(string audioType){
  
      if(audioType.compare("vlc")==0 ){
         advancedMusicPlayer = new VlcPlayer;
         
      }else if (audioType.compare("mp4")==0){
         advancedMusicPlayer = new Mp4Player;
      } 
   }

void play(string audioType, string fileName) {
   
      if(audioType.compare("vlc")==0){
         advancedMusicPlayer->playVlc(fileName);
      }
      else if(audioType.compare("mp4")==0){
         advancedMusicPlayer->playMp4(fileName);
      }
   }
};

class AudioPlayer : public MediaPlayer {

MediaAdapter* mediaAdapter; 
    
public:
  void play(string audioType, string fileName) {        

         if( (audioType.compare("mp3"))==0){
         cout << "Playing mp3 file. Name: " << fileName;            
      } 
       
      else if( (audioType.compare("vlc"))==0 || (audioType.compare("mp4"))==0){
         mediaAdapter = new MediaAdapter(audioType);
         mediaAdapter->play(audioType, fileName);
      }
      
      else{
         cout << "Invalid media. " << audioType << " format not supported";
      }
   }   
};

int main() {
      AudioPlayer *audioPlayer;
      audioPlayer->play("mp3", "beyond the horizon.mp3");
      audioPlayer->play("mp4", "alone.mp4");
      audioPlayer->play("vlc", "far far away.vlc");
      audioPlayer->play("avi", "mind me.avi");
   }

I don't think that "tutorialsoint" does such futile mistake. I've checked the link. It is not on C++ but Java maybe. be honest. — Itachi Uchiwa, Dec 07 '20 at 21:18
I advice you keep away from doing that because each language has its own convention among which memory management. for example C++ allow memory management through raw pointer will Java doesn't this will cause serious problems when dealing and "translating" from a language to the other. — Itachi Uchiwa, Dec 07 '20 at 21:25
@user10191234 See [this](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) and [this](https://stackoverflow.com/questions/3718998/fixing-segmentation-faults-in-c) please. — πάντα ῥεῖ, Dec 07 '20 at 21:33
Okay, a few comments. First, it will help a lot if you can identify where you're getting the segmentation violation. Either produce a core file and dump it, use a debugger, or add debug output to figure out where. Second, this is very strange polymorphism. Is there some reason your base class doesn't have a virtual void play() method, and your child classes each override that to play properly? Your USE of the object shouldn't have to if-check and call different player types. That's what polymorphism is supposed to do for you. — Joseph Larson, Dec 07 '20 at 21:23
@user10191234 *I said that I am translating Java to C++* -- You are trying to write C++ code using Java as a model, at least that is the impression from the code you've written. Do not do this, as trying to write C++ programs using Java as a model will either result in buggy programs, inefficient programs, or a combination of those two items. Maybe even worse, the programs will look nothing like an idiomatic C++ program, basically looking plain weird to a C++ programmer. This: `AudioPlayer *audioPlayer;` is an indication of this issue. All you need should do is `AudioPlayer audioPlayer;` — PaulMcKenzie, Dec 07 '20 at 22:07
@PaulMcKenzie, `AudioPlayer audioPlayer` didn't work either. — user10191234, Dec 07 '20 at 22:32
The point is that there are a lot of things you're doing that is making your code not work, and most of it is due to trying to write C++ code as if it's Java. There is no need for pointers for most of your program, including `mediaAdapter = new MediaAdapter(audioType);`. — PaulMcKenzie, Dec 07 '20 at 22:37
I get that it is not a helpful question, but I am not allowed to delete it. — user10191234, Dec 07 '20 at 22:39
After changing the `AudioPlayer audioPlayer;` and then using `audioPlayer.` instead of `audioPlayer->`, there are no compiler errors. The only errors are linker errors, as [seen here](http://coliru.stacked-crooked.com/a/c885d2651ee1c0e2). You need to actually provide the `play` functions you are calling for the program to actually pass the link phase. — PaulMcKenzie, Dec 07 '20 at 22:47
@PaulMcKenzie - I find most Java developers like to embellish their code a lot. It's usually overcomplicated. Over use of useless classes and other anti-patterns. — hookenz, Dec 08 '20 at 23:05

hookenz · Answer 1 · 2020-12-10T00:10:27.127

0

Good on you for learning C++.

"Segmentation fauled ... what did I do wrong to cause such an error." ?

Unfortunately, you've made a classic mistake in your program.

Right here. audioPlayer is a pointer. What do you think it points to?

AudioPlayer *audioPlayer // <---- The problem is here.  It's not initialised before you go to use it on the next line!
audioPlayer->play("mp3", "beyond the horizon.mp3"); // <--- It crashes here!

You have not initalised the pointer for the audioPlayer object.
It could point anywhere in memory. You haven't allocated an object to it. Something like this will get you further.

AudioPlayer *audioPlayer = new AudioPlayer();
// ...  do stuff
delete audioPlayer;  // Free it at the end.

Or if AudioPlayer is to exist for the lifetime of the app perhaps just

AudioPlayer audioPlayer;
audioPlayer.Play("mp3", "beyond the horizon.mp3");
// etc.  No need for new / delete.

edited Dec 10 '20 at 00:10

answered Dec 07 '20 at 21:26

hookenz

36,432
45
177
286

You should also show the OP how to free that memory once no mre needed otherwise the memory leaks. `delete audioPlayer`. such answers should be in comments I guess. – Itachi Uchiwa Dec 07 '20 at 21:27
I could... but I'm not here to re-write his code, just point him in the right direction. – hookenz Dec 07 '20 at 21:30
Half right direction I think. It is not about rewriting the code but as you know validating an invalid pointer ten leaking memory is another serious problem. – Itachi Uchiwa Dec 07 '20 at 21:31
1

`delete AudioPlayer();` ? are you serious? – Itachi Uchiwa Dec 07 '20 at 21:33
1

Settle down. It's a copy paste typo. – hookenz Dec 08 '20 at 00:14
With the current code status your suggestion won't work. – Joe Dec 09 '20 at 19:50
@Joe Op is asking the question "what did I do wrong to cause such an error." (seg fault) The answer is uninitialized pointer. Nothing much more needs to be said. I'm not critiquing his code. – hookenz Dec 09 '20 at 23:28

Adapter pattern in C++ returning a segmentation fault

1 Answers1