C++ iStream, If-Else & Vectors

Question

I have this iStream function to read in a .txt file of data in the format of:

  Pink Floyd: Dark Side of the Moon
  0:01:30 - Speak to Me

My function almost works perfectly.

Here are my problems that I just can't figure out. 1) It prints (outputs) as each track is added to an Album (so prints x copies of the first album, each with the next track attached. 2) When a new Album is created, it keeps the old tracks from the previous album and continues to add them one-by-one. (so the final album has every single track from all albums)

Obviously I want each album to only be printed once, and each album to only have its own songs... Any help would be appreciated. Thanks.

It would perhaps be better to ask this question at the [codereview](http://codereview.stackexchange.com) sister site. — David Hammen, Dec 11 '12 at 13:01

score 1 · Accepted Answer · answered Dec 11 '12 at 13:32

I see a number of problems with your code.

stringstream stringstreamFirst(line);

You aren't using the variable stringstreamFirst and line is empty at this point.

Album anAlbum(artistName,albumTitle,trackVector);

There are a number of problems with this line.

It's using the wrong values.
One the very first line, artistName, albumTitle, and TrackVector are empty. When you finally come across a new album, the artistName, albumTitle, and TrackVector are those for the previous album, not the current one. The values are correct by the time you get to the tracks on an album -- but that's not when you want to create a new album object.
It's in the wrong place.
As placed, this statement creates an Album object for each line in the input file. The right place to create a new album object is when you have encountered a new album entry in the input file.

stringstream stringstreamNew(line);
stringstream stringstreamNewNew(line);

Why the convoluted names, and why do you need two variables? An alternative would be to use but one stringstream, created as the first line of your while loop.

if (!(line[8] == '-'))
else if (line[8] == '-')

Don't replicate your boolean conditions like this. If you mean else (which is what you mean), just use else. The line is either an album entry or a track entry; there is nothing else.

else // These lines are missing

You don't have any error handling. What if you can't parse what presumably should be an album entry, or what presumably should be a track entry?

What you need to do (pseudocode):

while (getline(istr, line)) {
   stringstream linestream (line);
   if (line looks like an album line) {
      if (not the first line in the file) {
          // Create an album using the artist name, album title, and track vector
          // and add this album onto the vector of albums
      }
      // Parse the line for artist name and album title, preferably handling errors
      // Clear the trackVector that now pertains to the previous album
   }
   else {
      // Parse the line for track duration and name, preferably handling errors
      // Add the track to the track vector.
   }
}
// Create an album to cover the last album plus set of tracks

score 0 · Answer 2 · answered Dec 11 '12 at 13:15

Does Album perhaps store references (or even pointers) to its constructor arguments?

Don't do that, it means there's only ever just one vector of tracks - the one called trackVector - shared between albums, which keeps getting added to.

You should either have an addTrack member in Album or not construct an Album until you've read all the tracks depending on whether it makes sense for an Album to be mutable.

Also, you never modify aC - and what do you expect artistName = artistName to accomplish?

C++ iStream, If-Else & Vectors

2 Answers2