0

Novice / Student programmer, trying to figure out what's broken here.

I have a 'leg' class and a 'route' class, where the route is built by adding an earlier route object to a leg object.

The route class has two constructors; one to create the initial route using one leg, and one for building subsequent routes using the previous route plus another leg.

Everything seems to work fine for

  • creating the first route with the leg-only constructor

  • creating the second route with the leg+route constructor

  • creating the third route with the leg+route constructor

however, if i try to create a fourth route in the same manner I created the second and third, the program will compile, but will crash when the program reaches the first route object.

code as follows:

/////////////////////////////
/* includes and namespaces */
/////////////////////////////


#include <iostream>
using std::cin;
using std::cout;
using std::endl;

#include <ostream>
using std::ostream;


#include <cstdlib> // use for: atof(), atoi()




///////////////////////
/* const definitions */
///////////////////////




////////////////////////
/* struct definitions */
////////////////////////


///////////////////////
/* class definitions */
///////////////////////

class Route;

class Leg
{
  private:
    const char* const startCity;
    const char* const endCity;
    const int length;


  public: 
    Leg( const char* const, const char* const, const int );
    friend void printLeg( ostream&, const Leg&);


    friend void printRoute( ostream&, const Route& );

};

class Route
{
  private:
    const Leg** legsPtrArray;
    const int totalLegs;


  public:
    Route( const Leg& );
    Route( const Route&, const Leg& );
    Route( const Route&);  
    ~Route();

    friend void printRoute( ostream&, const Route& );



};


/////////////////////////
/* function prototypes */
/////////////////////////




///////////////////
/* main function */
///////////////////

int main()
{


  Leg Leg1( "San Francisco", "Reno", 218 );

  Leg Leg2( "Reno", "Salt Lake City", 518 );

  Leg Leg3( "Salt Lake City", "Kansas City", 604 );

  Leg Leg4( "Kansas City", "Indianapolis", 482 );

  Leg Leg5( "indianapolis", "NYC", 709 );

  cout << "Legs loaded, press [enter] to print each leg" << endl;

  cin.get();

  cout << "Leg 1: " << endl << endl;
  printLeg( cout, Leg1 );
  cin.get();

  cout << "Leg 2: " << endl << endl;
  printLeg( cout, Leg2 );
  cin.get();

  cout << "Leg 3: " << endl << endl;
  printLeg( cout, Leg3 );
  cin.get();  

  cout << "Leg 4: " << endl << endl;
  printLeg( cout, Leg4 );
  cin.get();  

  cout << "Leg 5: " << endl << endl;
  printLeg( cout, Leg5 );
  cin.get();

  cout << "Building complete route: " << endl << endl;

  Route Route1( Leg1 );

  Route Route2( Route1, Leg2 );

  Route Route3( Route2, Leg3 );

  cout << "Route built! Press [enter] to print" << endl << endl;

  cin.get();

  printRoute( cout, Route3);

  /*

  Route Route4( Route3, Leg4 );

  cout << "Route built! Press [enter] to print" << endl << endl;

  cin.get();

  printRoute( cout, Route4);

  */

  /*

  Route Route5( Route4, Leg5 );

  cout << "Route built! Press [enter] to print" << endl << endl;

  cin.get();

  printRoute( cout, Route5);

  */

  cout << endl;

  cout << "Press [enter] to quit " << endl;

  cin.get();


}

//////////////////////////
/* function definitions */
//////////////////////////


Leg::Leg( const char* const startCityToLoad, const char* const endCityToLoad, const int lengthToLoad )
 : startCity( startCityToLoad ), endCity( endCityToLoad ), length( lengthToLoad )
{

}

void printLeg( ostream& out, const Leg& legToPrint )
{
  out << "Leg start city: " << legToPrint.startCity << endl;
  out << "Leg end city: " << legToPrint.endCity << endl;
  out << "Leg length: " << legToPrint.length << " miles" << endl;

}

Route::Route( const Leg& legToAdd)
: totalLegs( 1 ), legsPtrArray ( new const Leg*[totalLegs] )
{

  legsPtrArray[0] = &legToAdd;

}

Route::Route( const Route& subRoute, const Leg& legToAdd)
: totalLegs( subRoute.totalLegs + 1 ), legsPtrArray ( new const Leg*[totalLegs] )
{

  for (int i = 0; i < subRoute.totalLegs ; i++)
  {
    legsPtrArray[i] = subRoute.legsPtrArray[i];

  }

  legsPtrArray[subRoute.totalLegs] = &legToAdd;

}

Route::Route( const Route& routeCopy )
: totalLegs( routeCopy.totalLegs ), legsPtrArray ( routeCopy.legsPtrArray )
{

}

Route::~Route()
{
  delete[] legsPtrArray;
}

void printRoute( ostream& out, const Route& routeToPrint )
{

  out << "Route: from " 
    << routeToPrint.legsPtrArray[0]->startCity 
    << " to "
    << routeToPrint.legsPtrArray[0]->endCity 
  ; 

  for (int i = 1; i < routeToPrint.totalLegs; i++)
  {
    out 
      << " to " 
      << routeToPrint.legsPtrArray[i]->endCity;
  }

  out << endl << endl;



  int routeLength = 0;

  for (int i = 0; i < routeToPrint.totalLegs; i++)
  {
    routeLength = 
      routeLength + routeToPrint.legsPtrArray[i]->length;

  }

    out << "Route Length: " << routeLength << " miles" << endl;

}

As-is, this compiles and runs fine.

If I remove the comments around

  /*

  Route Route4( Route3, Leg4 );

  cout << "Route built! Press [enter] to print" << endl << endl;

  cin.get();

  printRoute( cout, Route4);

  */

The program successfully creates the legs, then crashes when it reaches the first route.

I know I must be doing.... something wrong with the pointers, but I can't figure out what and why that would be causing the program to crash in this particular way.

If anyone has more insight than me I'd appreciate the help.

  • Is it crashing when creating the `Route` or when printing it? – Shane Haw Sep 30 '13 at 05:13
  • Not sure if it has any bearing on this crash, but your `Route` copy constructor is not correct. It will shallow copy, causing multiple deletes (undefined behavior) in the destructor. – Fred Larson Sep 30 '13 at 05:17

1 Answers1

0

Your problem is the order in which elements of the initializer list are constructed.

Try exchanging the member declarations in the Route class. It should look like this:

class Route
{
  private:
  const int totalLegs; // totalLegs comes first
  const Leg** legsPtrArray; // array comes next

I just verified it works on my system.

To understand why, look here.

EDIT: Adding the output I got:

Leg 5:

Leg start city: indianapolis
Leg end city: NYC
Leg length: 709 miles

Building complete route:

Route built! Press [enter] to print


Route: from San Francisco to Reno to Salt Lake City to Kansas City

Route Length: 1340 miles
Route built! Press [enter] to print


Route: from San Francisco to Reno to Salt Lake City to Kansas City to Indianapolis

Route Length: 1822 miles

Press [enter] to quit

EDIT: Explaining this quirk...

Any C++ class can have only one destructor, but several constructors.

Now, one of the requirements of a destructor is that it must destroy the fields of the object (primitive or otherwise) in the reverse order of their construction.

This implies there must be only one well-defined order of construction. Which means, the construction order of the fields cannot depend on the order in which they are specified in the constructor initialization list; because a class can have several constructors, with each specifying their own order of the fields in the initialization list. So the compiler uses the order of the fields in the class definition as the initialization order, and not the order from the initialization list.

In your program, basically the array legsPtrArray was being initialized before its length totalLegs (because of the order with which they were specified in the program). The undefined value of totalLegs will be some junk, which caused the initialization of the array to fail.

Community
  • 1
  • 1
Nikhil
  • 2,298
  • 13
  • 14
  • Dang, well I'm not understanding as of this moment why this is required, but it definitely works, so I'd better look at it until I figure out why it's required. Thanks! – stellars jay Sep 30 '13 at 05:44
  • @stellarsjay Updated my answer with some explanation. Hope this helps. – Nikhil Sep 30 '13 at 07:28
  • Okay yeah now it's making sense to me. I wasn't getting that the function took the declaration order from the class and not the initialization list. I guess it worked with the first three instances because the program was finding junk values that worked and using those. – stellars jay Sep 30 '13 at 23:05