Trouble implementing a function in a class

[Hengry]

I have to create a program that has a library of songs and a playlist contains songs from the library. My problem is the implementation of the Playlist class.

This is the code i have so far

  // This class keeps track of the songs names and identifies
class Song{

	public:
		// Default constructor
		Song(): songName(""), artistName(""), albumName("") {
			//Left blank intentionally
		}
		// Destructor	
		~Song() {};
		// Sets a name for the song
		void setSongName(string aSong){

			songName = aSong;
		}
		// Gets the name of the song
		string getSongName(){

			return songName;
		}
		// Sets the artist name of the song
		void setArtistName(string anArtist){

			artistName = anArtist;
		}
		// Gets the name of the artist
		string getArtistName(){

			return artistName; 
		}
		// Sets the album name of the song
		void setAlbum(string anAlbum){

			albumName = anAlbum;
		}
		// Gets the album name of the song
		string getAlbum(){

			return albumName;
		}

	private:
		string songName;
		string artistName;
		string albumName;
};

// This class has the ID and Number of plays of a song
// derived class of Songs, because it has Name, Artist, Album
class SongWrapper : public Song{

	public:
		// Default constructor
		SongWrapper() : iterativeID(0), numOfPlays(0){
			// left blank
		} 
		// Destructor
		~SongWrapper() {};

		//create an ID
		unsigned int createID(){

			return iterativeID++;
		}
		//Get the ID of the song
		unsigned int getID(){

			return iterativeID;
		}
		// Sets the number of plays of a song
		void setNumOfPlays(int plays){

			numOfPlays = plays;
		}
		// Gets the number of plays of a song
		unsigned int getNumOfPlays(){

			return numOfPlays;
		}

	private:
		unsigned int iterativeID;
		unsigned int numOfPlays;
};

//Contains all of the library's songs
//Name|Artist|Album|#ofPlays|identifier
class Library{

	public:
		// Default constructor
		Library(){}
		// Destructor
		~Library(){}
		//Create a song with Song details
		SongWrapper* createSong(string song, string artist, string album);
		// Plays a song a number of times
		unsigned int playSong(unsigned int numberOfPlays);
		// Checks if the song exist
		bool doesSongExist(string theSong);
		// Checks if the ID of the song exist
		bool doesIdentifierExist(unsigned int identifier);
		// Gets the details of the song
		string getSongDetails(SongWrapper* aSong);
		// Adds song into the library
		void addSongIntoLibrary(SongWrapper* aSong);
		// Remove song given the song ID number
		void removeSongFromLibrary(unsigned int identifier);

	private:
		unordered_map<int, SongWrapper*>aLibrary; // key = ID in the library
		unordered_map<string, int>libraryID; // key = Song name "Ex:Songname ArtistName AlbumName"

};

// Plays a song a number of times
unsigned int Library::playSong(unsigned int numberOfPlays){

	SongWrapper* aSong = new SongWrapper;
	//Checks if there is a song with the ID
	if(doesIdentifierExist(aSong->getID()) == false){
		return 0;
	}
	aSong->setNumOfPlays(aSong->getNumOfPlays() + numberOfPlays);
	return aSong->getNumOfPlays();
}

// Helper function to check if ID exist
bool Library::doesIdentifierExist(unsigned int identifier){

	if(aLibrary.find(identifier) == aLibrary.end()){
		cout << "No song with identifier #" << identifier << " exist in your library." << endl;
		return false;
	}
	return true;
}

// Helper function to check if there is a song with the specific song details
// Doesn't really have error checking yet
// If Name = The Cat, Artist = Meow, Album = One. String = The Cat Meow One
// If Name = The, Artist = Cat Meow, Album = One. String = The Cat Meow One
// Those would be accepted even though Name and Artist is different
bool Library::doesSongExist(string theSong){

	if(libraryID.find(theSong) == libraryID.end()){
		return false;
	}
	return true;
}

// Creates a song with song details
SongWrapper* Library::createSong(string song, string artist, string album){

	SongWrapper* aSong = new SongWrapper;
	aSong->setSongName(song);
	aSong->setArtistName(artist);
	aSong->setAlbum(album);
	aSong->setNumOfPlays(0);
	aSong->createID();
	return aSong;
}

// Return the song details as a string
string Library::getSongDetails(SongWrapper* aSong){

	string songString;
	songString = aSong->getSongName() + aSong->getArtistName() + aSong->getAlbum();
	return songString;
}

// Add the song into the library
void Library::addSongIntoLibrary(SongWrapper* aSong){

	string song,artist,album, theSong;
	theSong = getSongDetails(aSong);
	// Check if the song details already exist
	if(doesSongExist(theSong) == true){
		return;
	}
	// Adds into both maps
	pair<int, SongWrapper*>songWrapperPointer (aSong->getID(), createSong(song,artist,album));
	pair<string, int>songDetail (getSongDetails(aSong), aSong->getID());
	aLibrary.insert(songWrapperPointer);
	libraryID.insert(songDetail);
}

// Remove function
void Library::removeSongFromLibrary(unsigned int identifier){

	if(doesIdentifierExist(identifier) == false){
		return;
	}
	string songString;
	songString = getSongDetails(aLibrary.at(identifier));
	aLibrary.erase(identifier);
	libraryID.erase(songString);
}

This is what I have for my playlist class

// SongName|Artist|album
// Playlist has the: Song|Rating|#ofSongs in playlist
class Playlist{

	public:
		// Default constructor
		Playlist() : nameOfPlaylist(""), rating(1) {}
		// Destructor
		~Playlist() {}
		//Remove a playlist
		void removePlaylist();
		//Rename playlist
		void renamePlaylist();
		//Checks if playlist exists
		bool doesPlaylistExist(string playlistName);
		//Name the playList
		void namePlayList();
		// set name of playlist
		void setPlaylistName(string playListName){

			nameOfPlaylist = playListName;
		}
		// Get the name of playList
		string getPlaylistName(){

			return nameOfPlaylist;
		}
		unsigned int getRating(){

			return rating;
		}
		SongWrapper* getSongFromLibrary(unsigned int identifier);

	private:
                // Playlist name is the key, vector of songs are the values
		unordered_map<string, vector<SongWrapper*> >aPlaylist; 
                // Playlist name is the key, rating of the playlist is the value
		unordered_map<string, int> aPlaylistRating;
		string nameOfPlaylist;
		unsigned int rating;
};

The problem I'm running into is how I can keep track of the songs in my Library. If I create a libraryObject, the library will be blank. I'm thinking that I need a way to call my Library class in my Playlist class so that I have the list of songs in that library and I can add a song from my Library class into my Playlist class, but I'm not sure how to implement. Any help would be appreciated.

[48T7M4Gy]

Design a Library class with methods for adding and removing songs.

Do something similar for a Playlist class, the difference being perhaps you add song objects to it or just references.

PS If that's what your code does then you are on the right track. I see you used a <map> which is a good idea.

PPS. Have you tested it function by function with some reuseable test data? Does it work?

[gunnerfunner]

- lots of new without any delete => smart pointers
- Library::playSong() - new SongWrapper object created uses the default SongWrapper ctor and so its iterativeID = 0 by construction
- Library::createSong() - if SongWrapper inherited the Song ctor with the using declaration then it would be trivial to pass the strings song, artist, album to the Song part of the object w/o calling all the setters, numofPlays is already set to 0 by default and the way SongWrapper::createID() is written it seems you want a counter for the class objects which would be better handled by a static data member
- Library::getSongDetails - add some " " b/w song name, artist and alblum
- Library::addSongIntoLibrary - the strings song, artist, album declared but not initialized when they're passed to createSong()
- I'm still struggling to see what the SongWrapper class does - it tracks a song's ID and number of plays but presumably a song would be given and ID and played once it's part of the Library, no? In that case these data members can be subsumed within the Library class
-

The problem I'm running into is how I can keep track of the songs in my Library

you are building a Library (i.e. database) of Songs, wouldn't it be better to have this database even after the program ended? In that case I'd have a subclass of Songs called LibrarySongs with read, write, search methods for this class linked to the database and this class could also add the song's ID and number of plays that SongWrapper is doing now
- the relationship between Playlist and LibrarySongs is a has-a relationship - a playlist has a LibrarySong - so then you could consider containment or private inheritance

[Hengry]

@gunnerfunner
SongWrapper is essentially LibrarySong, and I have renamed my class accordingly. I have a to create a unique ID for each song, that's why I have an iterativeID++, but as you have mentioned, if i create an object the ID will default to 0. I am having trouble with static unsigned int iterativeID since I have functions createID() and getID().

LibrarySong has a unique ID that is created inside the Library class, but the LibrarySong is identifies itself with that ID, therefore I used unordered_map and made the key value the SongID number.

What other functions would you recommend me to add in LibrarySong?

I have changed my addSongIntoLibrary function to include getters.

void Library::addSongIntoLibrary(LibrarySong* aSong){

	string song,artist,album, theSong;
	song = aSong->getSongName();
	artist = aSong->getArtistName();
	album = aSong->getAlbum();
	theSong = getSongDetails(aSong);
	// Check if the song details already exist
	if(doesSongExist(theSong) == true){
		return;
	}
	// Adds into both maps
	pair<int, LibrarySong*>songWrapperPointer (aSong->getID(), createSong(song,artist,album));
	pair<string, int>songDetail (getSongDetails(aSong), aSong->getID());
	aSongInLibrary.insert(songWrapperPointer);
	libraryID.insert(songDetail);
}

I also included in my Library class a function that returns a LibrarySong* so that I am able to get a song from Library given the ID number. I am planning to create a Library object inside my functions such that I can use this function to get a song from the library and add it into my Playlist class

LibrarySong* Library::getSongFromLibrary(unsigned int identifier){

	if(aSongInLibrary.find(identifier) == aSongInLibrary.end()){
		cout << "No song with identifier #" << identifier << " exist in your library." << endl;
		return false;
	}
	return aSongInLibrary.at(identifier);
}

[gunnerfunner]

LibrarySong has a unique ID that is created inside the Library class

I think you meant to say 'inside the LibrarySong class'?

iterativeID has to be linked it to the static int data member in LibrarySong, let's call it counter, set to 0 initially. So whenever a LibrarySong object is created, iterativeID = counter + 1 and only then counter++, so when the first LibrarySong is created, counter == 0, so iterativeID is 1 and then counter becomes 1 so that next LibrarySong would be 2 and so on

What other functions would you recommend me to add in LibrarySong

- how does the data-member LibrarySong::numOfPlays work - is it changed everytime a LibrarySong makes it on to a Playlist? I suppose so, in that case you'd need some way of keeping track of the numOfPlays here

I hope you realize that in Library::libraryID there's no way of searching by just the song or artist or album name, it has to be (song name + artist name + album name) becaue of the way Library::getSongDetails() is written

It seems Library::aLibrary and Library::library are trying to be mirror images of each other with the keys, values swapped. In that case why don't you consider unordered_map<LibrarySong*, int>libraryID and that would make it easier to search as well

In your latest post, Library::addSongIntoLibrary(), line 15, aSongInLibrary ... this used to be aLibrary earlier

Think of the Library as a linked-list of LibrarySong objects, which in a sense it is, and that'd suggest that Library::createSong() is not a good idea, what you should have instead is Library::insertSong(). The Library does not create any LibrarySong objects, these are created in that class' ctor. This would also help you get rid of the problem of trying to set a LibrarySong's iterativeID from the Library class

Finally, a couple of questions of my own:
- how do you propose the Library and Playlist class interact
- what would your main() function look like i.e. what would be the general purpose of the program

ps: I'm also thinking if I might try a few things differently, if I can get anything working I'll put it on pastie and send you the link

[gunnerfunner]

OP: Below links to my class design. Three main changes b/w yours and mine:
- removed the link b/w _iterativeID and the order of object creation via the static data member of LibrarySong- if you really want to have an identifier linked to the time of creation of the object there are other ways to do this quite easily and gives greater flexibility
- the Library class is replaced by an actual file that will remain after the program ends and can be re-used
- LibrarySong has a private struct data-member, _libraryAccessor, that handles removeSong and getSong - this hides the database of library songs completely from users of the LibrarySong class who cannot make any unnecessary changes to it

As of now the header and implementaton files compile but obviously lots still left to be done. From here on it will depend on the relationship b/w LibrarySong and Playlist and the type of requirements of the main() program

header file: http://pastie.org/10983378
implementation file: http://pastie.org/10983379

[Hengry]

http://imgur.com/a/Pc3y2 This is the specs of my project that I would have to complete.

I have modified my "SongWrapper" class and named it "LibrarySong" for clarity.

class LibrarySong : public Song{

	public:
		// Default constructor
		LibrarySong(){

			++iterativeID;
			numOfPlays =0;
		} 
		// Destructor
		~LibrarySong() {};
		//Get the ID of the song
		static unsigned int getID(){

			return iterativeID;
		}
		// Sets the number of plays of a song
		void setNumOfPlays(int plays){

			numOfPlays = plays;
		}
		// Gets the number of plays of a song
		unsigned int getNumOfPlays(){

			return numOfPlays;
		}

	private:
		//static unsigned int counter;
		static unsigned int iterativeID;
		unsigned int numOfPlays;
};
unsigned int LibrarySong::iterativeID=0;

- I only have a default constructor at the moment because I was just thinking about using the ->get() to get the name and such.
- Instead of a counter, I just incremented the song ID by 1

- I propose that when a LibrarySong is created, it will create the Name, Artist, Album, #ofPlays, and the ID. I was thinking that createSong should be in the Library class because once you create the song, it's automatically added into your Library.

I modified my playSong function in my Library class so that it will return the total number of times the song has been played. I will do something similar in my Playlist class to get the number of songs played, since the numbers of time the songs are played are different in the two classes.

unsigned int Library::playSong(unsigned int identifier, unsigned int numberOfPlays){

	LibrarySong* aSong;
	//Checks if there is a song with the ID
	if(doesIdentifierExist(identifier) == false){
		return 0;
	}
	aSong = aSongInLibrary.at(identifier);
	aSong->setNumOfPlays(aSong->getNumOfPlays() + numberOfPlays);  
	return aSong->getNumOfPlays();
}

- My search function wouldn't require me to search for the name, artist, album, I would just need to search from the song ID number

- I changed the name of my first map so that I can better understand what it is actually mapping. That unordered_map maps a key to my SongLibrary*. So naming aLibrary wouldn't make sense because that's not my actual "Library" that stores all the SongLibrary would be created.

- I will create an actual "Library" containing all the other songs in another class, therefore I don't think I will need to create anything else in the LibrarySong class other than what I have already.

- When I was asked about Playlist class, I was given this answer.
“Library can expose an interface function that returns a (Library)Song* given a library song identifier. Keep in mind that Library and Playlists have dependencies in terms of songs, but they shouldn't really "know" about each other. Let the class that manages both the Library and all the Playlists take care of some of the system requirements (such as only adding a song to a Playlist if it exists in the library).”
- At the moment, the only functions I can think of inside my playlist class is createPlaylistName, changePlaylistName, ratePlaylist, doesPlaylistExist, numberOfSongsInsidePlaylist. Maybe I can get LibrarySong* from the Library class and add it into my Playlist? Then I would also be able to do removeSongFromPlaylist

- I created a function in my Library Class to get the LibrarySong* based on the ID of the song.

LibrarySong* Library::getSongFromLibrary(unsigned int identifier){

	return aSongInLibrary.at(identifier);
}

To answer your question:

- I would store my LibrarySong* into a vector (perhaps) and that will become my "Library" in another class maybe called Driver. The purpose of the Driver class is being sort of like a "bridge" between the Library and the Playlist classes.
- My main() will just be the list of Commands that the user inputs and that's pretty much it.

[48T7M4Gy]

FWIW, by referring to LibrarySong instead of Library, being a collection of Songs there is a danger that you will have great difficulty down the track in decoupling the two quite distinct classes Library and Song. A Library has a completely different role to a Song.

Consider if you have a Song that you want to allocate to 5 Library's. You will have to go to each Library and create 5 new Songs. By decoupling, you create one Song and simply add the same Song to each of the 5 Library's. Without decoupling the two, editing the details of the Song is a similar nightmare.

[Hengry]

@kemort I understand what you mean, but I would assume that there is only 1 "Library", where it's the master list of all the songs and many playlists that pull songs from the "Library"

[48T7M4Gy]

:)

[gunnerfunner]

I was thinking that createSong should be in the Library class ...

createSong creates a Song object so I think it's fine within the Song or LibrarySong class and
Library::addSongIntoLibrary() is already doing the job of adding songs into Library

once you create the song, it's automatically added into your Library

in that case there is no difference between class Song and class Library

aSong->setNumOfPlays(aSong->getNumOfPlays() + numberOfPlays);

here, wouldn't getNumOfPlays() be equal to numberOfPlays? the idea is good but the last term of this statement should be linked to Playlist::numberOfPlays so that we have an updated total Library::numberOfPlays

- the PLY command in your assignment has two parameters - the song and # of times to play it - the assignment says that this command should be in the user's library - does this mean a Playlist according to your definition or would songs be played directly from the Library class

Let the class that manages both the Library and all the Playlists ... the Driver class is being sort of like a "bridge" between the Library and the Playlist classes

at first I felt that Driver was required but what if Library had a member vector<Playlists>? So in one place you have all the information you need about the Playlists and the songs, and in effect the Library class becomes the Driver. In that case commands RP, ASP, RSP, RT, PLS, AG, EXP can all be in the Library class and you need to add the requirements of the LB command to your existing Library class

- Remember to const qualify, pass-by-reference, delete objects created with new or using smart pointers etc wherever applicable as you get towards finalising the draft