0

I'm making a program that would essentially just be a manager for a record label (I'm just kind of making stuff up as I go along for fun), and I have a few classes. class RecordLabel, class Rapper, and class Album so far. Here's album:

using namespace std;
#include <iostream>
#include <string>

class Album
{
public:
    void getAlbumName() const;
    string setAlbumName(string);
    void getNumTracks() const;
    int setNumTracks(int);
    void getTracklist() const;
    string setTracklist(string);
private:
    string albumName;
    int numTracks;
    string tracklist[];
};

So I'm just not sure about tracklist. My first idea was to use an array as you can see, but honestly I'm not sure what would be best. Could I use a linked list? Should I use an array of pointers? I've always been somewhat iffy on my knowledge of things like pointer arrays and lists so I figured this was a good opportunity to learn something.

Edit: I should be more clear: tracklist would just be a way to store the titles of however many tracks on an album.

genpfault
  • 51,148
  • 11
  • 85
  • 139
SaucedCaveman
  • 41
  • 2
  • 9
  • 1
    It depends what you want to do with it. If your goal is only to store the strings without any form of sorting, searching and stuff, I'd usually go for a `std::vector` that'll give you iterators, range-based loops and all – Fluffy May 29 '18 at 22:16
  • 2
    This is going to get closed, but I would observe that things like album name (and tracks) don't change, and should be set once, in the album class's constructor. Setters are almost always a code smell. –  May 29 '18 at 22:18
  • 1
    Please [don't use `using namespace std;`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). – Max Vollmer May 29 '18 at 22:18
  • 1
    [OT] your getters returns nothing and setters don't return void – Fluffy May 29 '18 at 22:18
  • @Fluffy I'd say the only sorting I'd be looking for is just the order they would be on a given album. Which I guess would either be sorted by the order I enter the names, or by number if I prefix each title with a track number. Does what you suggested still work for that? I've honestly never really worked with vectors. – SaucedCaveman May 29 '18 at 22:21
  • @NeilButterworth Well... An album that hasn't been released yet could undergo a lot of changes pre-release. Obviously it's all I hypothetical so I could just assume it wouldn't for simplicity's sake. I haven't forgotten about the constructor, just haven't put it in yet. Appreciate the reminder. – SaucedCaveman May 29 '18 at 22:25
  • @Sauced In that case, create a new album instance. You really want your objects to be immutable if at all possible. –  May 29 '18 at 22:25
  • @NeilButterworth Interesting. I see someone else suggested not using setters either. Is that related to what you just said, that objects shouldn't really change? I'm a student and I just write code the way I was taught, so it never really occurred to me how potentially impractical the way we write code in a classroom just might be in the real world haha. – SaucedCaveman May 29 '18 at 22:32
  • @Sauced Ideally, objects should never change their properties (this is how pure functional programs work). Procedural languages like C++ tend to be a bit more pragmatic in this regard, but consider - if you can change all the properties of an object, is it in any sense the same object you started out with? And yes, setters are generally considered to be things to be avoided, where possible. –  May 29 '18 at 22:36
  • There is an art to using setters. A `public` setter that does nothing but set is no better than a `public` member as far as encapsulation is concerned. If any fool can set any value, what's the point? A setter should validate the input in some way or perform some support task on set. If it doesn't it's just a place to hang a breakpoint when debugging. – user4581301 May 29 '18 at 23:00

1 Answers1

0

Firstly, using namespace std can lead to subtle bugs, as mentioned by Max Vollmer in the comments.

I'd prefer setting to be done in the constructor, there won't be much reason to keep changing properties like Name.

This looks to be classic OOP, you have an 'Album' which is a collection of 'Tracks', and these tracks have both RecordLabel and Artist? Say, a Artist can be Emimem, type 'Rapper', genre 'Hip Hop'; and a RecordLabel can be Vanity, Open ect.

So, lets say, something like this -

class RecordLabel
{
  LableType m_Type;
  public:
  RecordLabel():m_Type(LableType::None) {}
  RecordLabel(LableType L)
  {
     m_Type = L;
  }
};

class Artist
{
  std::string m_Name;
  ArtistType  m_Type;
  ArtistGenre m_Genre;
public:
  Artist():m_Type(ArtistType::None), m_Genre(ArtistGenre::None) {}
  Artist(const std::string Name, ArtistType Type, ArtistGenre Genre)
  {
    m_Name = Name; m_Type = Type; m_Genre = Genre;
  }
};

class Track
{

std::string m_TrackName;
Artist      m_Artist;
RecordLabel m_Lable;
public: 
Track(const std::string Name, const Artist& A, const RecordLabel& R)
{
    m_TrackName = Name;
    m_Artist    = A; 
    m_Lable     = R;
}

friend ostream& operator<<(ostream& os, const Track& dt)
{
    std::cout<<"TrackName: "<<dt.m_TrackName;
    return os;
}
};

class Album
{
std::string        m_albumName;
std::vector<Track> m_Tracks;

public: Album(const std::string& Name)  
    {
        m_albumName = Name;
    }

void AddTrack(const Track& T)
{
    m_Tracks.push_back(T);
}

friend ostream& operator<<(ostream& os, const Album& dt)
{
    cout<<"Album Name: "<<dt.m_albumName<<"\n";
    for(auto It: dt.m_Tracks)
    {
        std::cout<<It<<"\n";         
    } return os;
}
};

Working code here.