2

Some C++ source code I wrote compiled and ran fine on Windows 7 using Visual Studio 2010. But when I tried it on Mac OS X 10.8 Mountain Lion using XCode 4.4.1, it would not compile. I was able to fix it as detailed below, but am interested to find out what I was doing wrong in the first place and why the fix was needed.

Specifically, consider the following class declaration:

class airline {
    vector <passenger> persons;
    int npassengers;
public:
    airline () {};
    airline (int np);
    ~airline () {};
    // other functions
} ;

The constructor for airline that takes an int parameter is:

airline::airline (int np) {
    npassengers = np;
    persons.resize (npassengers);
    return;
}

On the Windows machine, this compiled and ran with no problems. However, XCode complained bitterly that I did not have a viable constructor for "passenger". I have a default constructor for passenger, so that wasn't the issue. Based on something I found in an earlier question on this site I also tried using persons.erase instead of resize, with no luck.

Ultimately I was able to get the code to compile on the Mac by using persons.vector::resize (npassengers); instead of persons.resize (npassengers);. I am wondering why it was necessary to do that on the Mac side, and why it worked without a problem on the Windows side.

A related question is, is it bad practice to have using namespace std; at the top of your code, and instead, should I be specifying std::cout etc.? If so, why?

Please bear with my lack of knowledge, and thanks for your help!

Edit: Sean suggested I show my code for passenger, so here goes:

class passenger {
    string plast;
    string pfirst;
    int  row;
    char seatno;
    int flightno;
public:
    passenger ();
    passenger(string line );
    passenger (passenger const &rhs);
    ~passenger() {};
} ; 

passenger::passenger (passenger const &rhs) {
    plast = rhs.plast;
    pfirst = rhs.pfirst;
    row = rhs.row;
    seatno = rhs.seatno;
    flightno = rhs.flightno;

    return;
}

passenger::passenger( string line ) {
    // tokenizes string to obtain passenger data
    // seems to work fine, actual code is too long to include
    // and doesn't seem relevant here
}

passenger::passenger() {
    plast.clear();
    pfirst.clear();
    row = 0;
    seatno = '\0';
    flightno = 0;

    return;
}

Plus of course the usual accessors and overloaded insertion operator, etc. Thanks!

Community
  • 1
  • 1
verbose
  • 7,827
  • 1
  • 25
  • 40
  • 1
    You gave no code for passenger, which might be affecting things. also, why are you storing npassengers? persons.size() gives you the exact same thing, and you're just wasting space and time by duplicating a member variable already inside vector. – Sean Middleditch Aug 21 '12 at 04:31
  • The vector starts off empty, and npassengers is what determines how many passengers get stored in it. The number of passengers varies from one airline to the next. I'll edit to show some of the passenger code. – verbose Aug 21 '12 at 05:03
  • It is bad practice to use `using namespace std`, specially in headers. It just makes it very likely that you will get name clashes, and these name clashes can pop up just by including another C++ standard library header. I would say it is not worth the few characters it saves you from typing. See [here](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-a-bad-practice-in-c). Some people will say it is OK to use it in `cpp` files, and I will never agree with that. – juanchopanza Aug 21 '12 at 05:09
  • 1
    @SeanMiddleditch: I agree but the real problem is not space and time but the fact that we have duplicate information that may become out of sync with each other. – Martin York Aug 21 '12 at 05:10
  • There is no need for the copy constructor in `passengers`. The compiler-synthesized one will do just fine in this case. Also no need to call `clear()` on the string data members in the default constructor, where you could also use the initialization list instead of assigning values in the constructor body. – juanchopanza Aug 21 '12 at 05:18
  • Thanks @juanchopanza. I will keep those tips in mind for future. – verbose Aug 21 '12 at 05:23
  • I wonder if initializing `npassengers` in the initializer list and then resizing the `vector` in the body of the constructor would change anything. In it's beginning days, C++ initializer lists were way more efficient performance-wise than doing the work in the body of the constructor. It seems the rules were a little different too. – JimR Aug 21 '12 at 05:27
  • @JimR much better to initialize the vector with `np` in the initialization list, and remove the `npassengers` data member all together. – juanchopanza Aug 21 '12 at 05:28
  • @juanchopanza: Agreed and that might fix the problem... `npassengers` is after `vector` in the data of the class. I remember having problems in that case, at least with early IBM, Borland and Microsoft C++ compilers. I'm thinking that maybe XCode has some leftover stupidity that could be exposed here... It was just a fleeting thought. :) – JimR Aug 21 '12 at 05:34
  • @JimR it won't fix the problem, it is just better design. – juanchopanza Aug 21 '12 at 05:40

1 Answers1

1

I cannot see any obvious reason why the code would not be portable, other than missing header files or problems with other code not shown in the question. I can suggest some lines of portable code with some improvements:

#include <vector>
#include "passenger.h" // assuming class passenger is declared here

class airline {
    std::vector <passenger> persons_; // trailing underscore to distinguish data members
public:
    airline () {};
    explicit airline (int np); // explicit constructor to avoid implicit conversion from int.
~airline () {};
// other functions
} ;

And in the .cpp file:

airline::airline (int np) : persons_(np) {} // persons has np passengers

You don't need a separate data member for the number of passengers. You can get it from std::vector::size().

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • Thanks @juanchopanza. What fixed it was explicitly including `` in the .cpp file, not just in a different header file. I am still puzzled about why it was necessary to do so; but presumably there is another `resize` function that gets called if `` is not explicitly included in the .cpp file. The rest of your suggestions are very helpful too, particularly about how to write the constructor for the airline so as to get rid of `npassengers` – verbose Aug 21 '12 at 07:05