Using dynamic memory crashes program in C++

Question

My program is running fine with standard memory allocation but using dynamic memory allocation makes the program act weird! It doesnt print anything even from main function.

Have declated pointer to alist in header file and have dynamically allocated memory using new in class defination(constructor). Have pointed the code with witch program works fine. Also have used many couts for testing, ignore them. Thanks

Header file:

//Vehicle.h
//Author: Ankush Kaul
//Purpose: Declaration of Vehicle class

#ifndef VEHICLE_H
#define VEHICLE_H

#include <string>
#include <fstream>

using namespace std;


class Vehicle
{
public:
    Vehicle(); //default constructor
    Vehicle(ifstream&); //parametrized constructor
    Vehicle(const Vehicle& other); //copy constructor
    ~Vehicle(); // destructor

    //over loaded operators
    Vehicle& operator=(const Vehicle& other); // assignment operator
    bool operator==(const Vehicle& other); // relational operator

    // accessors
    int getVid();
    string getManufact();
    string getColor();
    double getCost();
    int getnumAccessories();
    string getList();

    // 3 functions to access accessory list
    void startAcc();
    string nextAcc();
    bool hasNextAcc();

protected:

private:
    int vId;
    string manufact;
    string color;
    double cost;
    int numAccessories;
    string *alist; // acessory list 
    //string alist[10]; <- WORKS WITH THIS
    int counter; // hidden object used for accessing accesory list
};

#endif // VEHICLE_H

Class definition:

//Vehicle.h
//Author: Ankush Kaul
//Purpose: Implementation of Vehicle class

#include "Vehicle.h"
#include <cstdlib>
#include <fstream>
#include <iostream>

using std::atoi;
using std::atof;
using std::ifstream;
using std::cout;


//---------------------------
//Name: Vehicle
//Purpose: default constructor
//Parameters: None
//Return: None
//---------------------------
Vehicle::Vehicle()
{
    vId = 999;
    manufact = color = "";
    cost = numAccessories = 0;
}

//---------------------------
//Name: Vehicle()
//Purpose: Parametrized constructor
//Parameters:
//      fin : input file stream
//Return: None
//---------------------------
Vehicle::Vehicle(ifstream &fin)
{
    string line;

    getline(fin, line);
    vId = atoi(line.c_str());

    getline(fin, line);
    manufact= line;

    getline(fin, line);
    color = line;

    getline(fin, line);
    cost = atof(line.c_str());

    getline(fin, line);
    numAccessories = atoi(line.c_str());

    alist = new string[numAccessories]; <-- WORKS WITHOUT THIS
    cout<<"**";

    for (int i = 0; i < numAccessories; i++)
    {
        getline(fin, line);
        alist[i] = line;
    }

}

//---------------------------
//Name: ~Vehicle
//Purpose: Destructor
//Parameters: None
//Return: None
//---------------------------
Vehicle::~Vehicle()
{
}

//---------------------------
//Name: Vehicle()
//Purpose: Copy constructor
//Parameters:
//      other : Vehicle object passed to copy
//Return: None
//---------------------------
Vehicle::Vehicle(const Vehicle& other)
{
    this->vId = other.vId;
    this->manufact = other.manufact;
    this->color = other.color;
    this->cost = other.cost;
    this->numAccessories = other.numAccessories;

    for ( int i = 0 ; i < other.numAccessories ; i++ )
        this->alist[i] = other.alist[i];
}

//---------------------------
//Name: operator=
//Purpose: Overloading operator
//Parameters:
//      other : Vehicle object passed to assign values
//Return: Vehicle
//---------------------------
Vehicle& Vehicle::operator=(const Vehicle& other)
{
    this->vId = other.vId;
    this->manufact = other.manufact;
    this->color = other.color;
    this->cost = other.cost;
    this-> numAccessories = other.numAccessories;

    for ( int i = 0 ; i < other.numAccessories ; i++ )
        this->alist[i] = other.alist[i];

    return *this;
}

//---------------------------
//Name: operator==
//Purpose: Overloading operator
//Parameters:
//      other : Vehicle object passed to compare
//Return: bool
//---------------------------
bool Vehicle::operator==(const Vehicle& other)
{
    if (this->vId == other.vId &&
         this->manufact == other.manufact &&
          this->color == other.color &&
           this->cost == other.cost &&
            this->numAccessories == other.numAccessories)
    {
        return true;
    }

    return false;
}

//---------------------------
//Name: getVid
//Purpose: gets vId attribute
//Parameters: none
//Return: int
//---------------------------
int Vehicle::getVid()
{
    return vId;
}

//---------------------------
//Name: getManufact
//Purpose: gets manufact attribute
//Parameters: none
//Return: string
//---------------------------
string Vehicle::getManufact()
{
    return manufact;
}

//---------------------------
//Name: getColor
//Purpose: gets color attribute
//Parameters: none
//Return: string
//---------------------------
string Vehicle::getColor()
{
    return color;
}

//---------------------------
//Name: getCost
//Purpose: gets cost attribute
//Parameters: none
//Return: double
//---------------------------
double Vehicle::getCost()
{
    return cost;
}

//---------------------------
//Name: getnumAccessories
//Purpose: gets numAccessories attribute
//Parameters: none
//Return: int
//---------------------------
int Vehicle::getnumAccessories()
{
    return numAccessories;
}

// 3 functions to get accesory list

//---------------------------
//Name: startAcc
//Purpose: sets counter to zero
//Parameters: none
//Return: void
//---------------------------
void Vehicle::startAcc()
{
    counter = 0;
}

//---------------------------
//Name: nextAcc
//Purpose: returns name of accesory
//Parameters: none
//Return: string
//---------------------------
string Vehicle::nextAcc()
{
    return alist[counter++];
}

//---------------------------
//Name: hasNextAcc
//Purpose: checks if all accessory list is read
//Parameters: none
//Return: bool
//--------------------------
bool Vehicle::hasNextAcc()
{
    if ( counter < numAccessories)
        return true;

    return false;
}

Main file:

//main.cpp
//Topic: Project 4 - Vehicle
//Author: Ankush Kaul
//Purpose: Read and display list of Vehicles

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>
#include <iomanip>
#include "Vehicle.h"

using namespace std;

void printVehicle (Vehicle);

int main ()
{

    ifstream fin; // holds input file stream
    string file,line; // file - holds filename, line - holds line read from file

    cout<<"Enter filename : ";
    cout<<"()";
    //cin>>file;
    cout<<"11";
    //fin.open(file.c_str());
    fin.open("vehicle1.txt");
    cout<<"^^";
    // checking if file is sucessfully opened
    if (!fin.is_open())
    {
        cout<<"Error opening file!";
        return 1;
    }
    cout<<"@@";

    //cin.ignore(1000, '\n');
    cout<<"%%";
    getline(fin, line); // reading the first line which contains number of vehicles in the file
    cout<<"!!";
    int count = atoi(line.c_str());
    cout<<"&&";
    Vehicle veh[count]; // defining a array of length = count

    // loop to read details of vehicles from file and store it in the veh[] array
    for (int i = 0; i < count; i++)
    {
        cout<<"##";
        Vehicle newVeh(fin);

        bool duplicate = false; // sets duplicate flag to false

        // loop to check if a duplicate vehicle exists
        for (int k = 0; k < i; k++)
            if (newVeh == veh[k])
            {
                duplicate = true;
                cout<<endl<<"Duplicate Vehicle detected --- ignored!"<<endl;
            }

        if (duplicate)
            continue;

        veh[i] = newVeh;
    }

    // print list of vehicles to the screen by calling printVehicle function
    cout<<endl<<"Vehicle List";

    for (int i = 0; i < count; i++)
    {
    //cout<<endl<<"in loop "<<i<<endl;
    cout<<endl<<"-------------------------";
    printVehicle(veh[i]);
    }

    return 0;
}

//---------------------------
//Name: printVehicle
//Purpose: prints the vehicle details
//Parameters: veh
//Return: void
//---------------------------
void printVehicle(Vehicle veh)
{
    cout<<endl<<"Vin: "<<veh.getVid();
    cout<<endl<<"Manufacturer: "<<veh.getManufact();
    cout<<endl<<"Color: "<<veh.getColor();
    cout<<fixed<<showpoint<<setprecision(2); // output modifier to print decimal with 2 point accuracy
    cout<<endl<<"Cost: "<<veh.getCost();
    cout<<endl<<"Accessory List:"<<endl;

    // printing accessory list
    veh.startAcc();
    while(veh.hasNextAcc())
    {
        string nextAcc = veh.nextAcc();
        cout<<"  "<<nextAcc<<endl;
    }

}

Answer 1

Your copy constructor and assignment operator are broken. You are not allocating memory for the accessory list. Your copy constructor should look something like this:

Vehicle::Vehicle(const Vehicle& other)
:
    vId (other.vId),
    manufact (other.manufact),
    color (other.color),
    cost (other.cost),
    numAccessories (other.numAccessories),
    alist (0),
    counter (0)
{
    alist = new string[numAccessories]; 
    for ( int i = 0 ; i < other.numAccessories ; ++i ) {
        alist[i] = other.alist[i];
    }
}

Your assignment operator should similarly allocate alist before assigning to it. There's another issue with your assignment operator that you aren't addressing, which is the self-assignment problem. What if someone does vehicle = vehicle; ?

Your destructor is also incorrect. You should be releasing the memory allocated by the constructor or assignment operator.

Your comparison operator isn't broken, but it too may well be incorrect. Presumably you should be testing that the two vehicles have the same accessories rather than just testing that they have the same number of accessories.