Strange behavior for int / counter

Question

I'm trying to add some songs to a vector inside a class. One of the values I'm storing is an int representing the song. It's essentially a counter. The first song I add should have the value 1, the second value two and so forth. But It's getting other strange values like big random numbers (positives and negatives). I can't wrap my head around what I'm doing wrong. This is the code:

#include <iostream>
#include <vector>
#include <string>

using namespace std;

class Jukebox{
public:
  void addSong(string artist, string title, string filename) {
    song s {++songCounter, artist, title, filename};
    Songs.push_back(s);
  }

  void printSong (int song) {
    cout << Songs[song].no << ". ";
    cout << Songs[song].artist << " - ";
    cout << Songs[song].title << " : ";
    cout << Songs[song].filename << endl;
  }

private:
  struct song {
    int no;
    string artist;
    string title;
    string filename;
  };
  vector<song> Songs;
  int songCounter;
};

int main() {
  Jukebox jbox;
  jbox.addSong("U2", "Magnificent", "U2-Magnificent.mp3");
  jbox.addSong("Sting", "Englishman in New York", "Sting-Englishman_in_New_York.mp3");
  jbox.addSong("U2", "One", "U2-One.mp3");
  jbox.printSong(0);
  jbox.printSong(1);
  jbox.printSong(2);
  return 0;
}

Update

Ok, I'm probably stupid and should read more about classes before trying to implement this. But I think I did read and I still don't get it. This is what my class looks like now (which won't work):

class Jukebox(): songCounter(0)
 {

public:
  void addSong(string artist, string title, string filename) {
    songCounter++;
    song s {songCounter, artist, title, filename};
    Songs.push_back(s);
  }

  void printSong (int song) {
    cout << Songs[song].no << ". ";
    cout << Songs[song].artist << " - ";
    cout << Songs[song].title << " : ";
    cout << Songs[song].filename << endl;
  }

private:
  int songCounter;
  struct song {
    int no;
    string artist;
    string title;
    string filename;
  };
  vector<song> Songs;
};

Final word

Ok. From the example I've seen of c++ contructor classes I had some kind of wrong impression of how they worked. Now I think I'm getting it a little bit more. But the syntax still seems strange to me. But I try to read more so I really understand it. Here is what I did and to seems to work:

#include <iostream>
#include <vector>
#include <string>

using namespace std;

class Jukebox {

public:
  void addSong(string artist, string title, string filename) {
    songCounter++;
    song s {songCounter, artist, title, filename};
    Songs.push_back(s);
  }

  void printSong (int song) {
    cout << Songs[song].no << ". ";
    cout << Songs[song].artist << " - ";
    cout << Songs[song].title << " : ";
    cout << Songs[song].filename << endl;
  }

  Jukebox(): songCounter(0) {} // Constructor

private:
  int songCounter;
  struct song {
    int no;
    string artist;
    string title;
    string filename;
  };
  vector<song> Songs;
};

int main() {
  Jukebox jbox;
  jbox.addSong("U2", "Magnificent", "U2-Magnificent.mp3");
  jbox.addSong("Sting", "Englishman in New York", "Sting-Englishman_in_New_York.mp3");
  jbox.addSong("U2", "One", "U2-One.mp3");
  jbox.printSong(0);
  jbox.printSong(1);
  jbox.printSong(2);
  return 0;
}

Alok Save · Accepted Answer · 2012-01-14T21:09:16.350

4

You did not initialize songCounter in your constructor.

Jukebox(): songCounter(0),//....other members

If you do not initialize it, then it may have any random value and that leaves your program in an Undefined State.

Always be careful while using unitialized variables, it often leads to Undefined Behavior and your program is a good example of it.

Also, I am not sure of your design but probably it should be a static member if you want to use it as a counter, which maintains state for all objects of your Song class.
Or
You will have to explicitly set it to a proper value at time of creating a Song object.

Okay its a counter for JukeBox and not Song class so its still okay to be a member.

edited Jan 14 '12 at 21:09

answered Jan 14 '12 at 21:00

Alok Save

202,538
53
430
533

it doesn't have to be static, it's perfectly fine as a member of `Jukebox`, but it does need to be intialized.. – Nim Jan 14 '12 at 21:03
Sorry to ask this of you. But can you extend your code snippet a little? Because I don't get it. Knowing only python before, `songCounter(0)` seems like it means calling function named songCounter, but it is a variable. When I try doing what I think you mean it trows an error. I could update the question if you want to know what I've tried. – Niclas Nilsson Jan 14 '12 at 21:26
@NiclasNilsson: No problem. Please check out this answer of mine in C++ FAQ [here](http://stackoverflow.com/a/8523361/452307) it explains in detail your doubt. – Alok Save Jan 14 '12 at 21:33
Oh thanks. I updated the question. But I'll check you FAQ out before asking for more help :-) – Niclas Nilsson Jan 14 '12 at 21:34
@NiclasNilsson: I saw your updated Q and I see whats wrong in it Also, I am assured the answer link I provided you points out the same.So happy learning :) – Alok Save Jan 14 '12 at 21:37
Yeah it did. And that's the whole point of this code. I'm trying to (more or less) convert a python program of mine just for sake of learning (even thought I doubt I will finish the whole program before I'm trying to code something diffrent) :-) – Niclas Nilsson Jan 14 '12 at 21:48

score 1 · Answer 2 · answered Jan 14 '12 at 21:01

1

You didn't initialize the variable songCounter.

Add the following to the class definition of Jukebox:

Jukebox(): songCounter(0) {}

answered Jan 14 '12 at 21:01

celtschk

19,311
3
39
64

Yeah. I get it now. Voting you up! – Niclas Nilsson Jan 14 '12 at 21:43

Stuart Golodetz · Answer 3 · 2012-01-14T21:48:32.113

1

Where do you initialise songCounter? In C++, primitives aren't zero initialised by default. You need to add

: songCounter(0)

to your constructor.

edited Jan 14 '12 at 21:48

answered Jan 14 '12 at 21:01

Stuart Golodetz

20,238
4
51
80

Inialization of variables in c/c++ does not depend on their types(whether they are primitives or non primitives) it depends on the storage class(`static`,`register` etc) of the declared variable. – Alok Save Jan 14 '12 at 21:20
Default initialisation involves calling the default constructor for class types, default initialising all the array elements for array types, and doing nothing otherwise (e.g. for primitives). Statics are zero initialised, which is different. – Stuart Golodetz Jan 14 '12 at 21:43
Yeah. I get it now. Voting you up! – Niclas Nilsson Jan 14 '12 at 21:43

score 1 · Answer 4 · answered Jan 14 '12 at 21:02

1

you need a constructor for Jukebox and in that you need to initialise the counter to 0.

answered Jan 14 '12 at 21:02

Nim

33,299
2
62
101

Yeah. I get it now. Voting you up! – Niclas Nilsson Jan 14 '12 at 21:44

score 1 · Answer 5 · answered Jan 14 '12 at 21:02

1

I think you should initialize songCounter to be 0. In the public part of the class:

public Jukebox() : songCounter(0) {}

answered Jan 14 '12 at 21:02

ronme

1,154
1
11
18

Yeah. I get it now. Voting you up! – Niclas Nilsson Jan 14 '12 at 21:45

Strange behavior for int / counter

Update

Final word

5 Answers5