-1

I've created a class Student which contains a dynamic array. I've filled the first two items with the constructor. Every method I've tried to use to access/print those two elements from the main get a read/access violation and crashes. I've added a cout in the constructor that shows the elements ARE filled and exist. I've included two failed methods in the main: A void function that attempts to send first element to cout, and a method that accepts an int for the desired index. Both have been commented out to allow a test run showing elements are created and printed by the constructor.

Header:

#ifndef STUDENT_H
#define STUDENT_H
#include <string>
#include <iostream>
#define ARRAY_MAX 15
using namespace std;

class Student
{
private:
    string firstName, lastName;
    unsigned int ID, numItems = 0;
    typedef string* StringPtr;
    StringPtr items;

public:
    int capacity = 15;
    Student();
    Student(const string fName, const string lName, const unsigned int id);
    string getfName() const;
    string getlName() const;
    void getItem(int num);
    string getItemB(int num) const;
    unsigned int getID() const;
};
#endif // STUDENT_H

Definitions:

#include "student.h"
using namespace std;

Student::Student()
{
}

Student::Student(const string fName, const string lName, const unsigned int id)
{
    firstName = fName;
    lastName = lName;
    ID = id;

    StringPtr items = new string[capacity];
    numItems = 0;
    items[0] = "stuff";
    items[1] = "things";
    cout << items[0] << endl << items[1] << endl;
}

string Student::getfName() const
{
    return firstName;
}

string Student::getlName() const
{
    return lastName;
}

void Student::getItem(int num)
{
    cout << items[0] << endl;
}

string Student::getItemB(int num) const
{
    return items[num];
}

unsigned int Student::getID() const
{
    return ID;
}

Main:

#include <iostream>
#include <fstream>
#include <string>
#include <iomanip>
#include "student.h"
using namespace std;



int main()
{
    Student stu;
    string str;

    stu = Student("John", "Smith", 1200);
    cout << stu.getfName() << " " << stu.getlName() << endl;
    //stu.getItem(0);
    //cout << stu.getItemB(0);

    system("pause");

    // Quit without error
    return 0;
}
Justin Scott
  • 1
  • 1
  • 2
  • You have to do a `deep copy` not just `shallow` by overloading the assignment operator `=`. – Raindrop7 Mar 12 '18 at 18:27
  • Have you tried debugging through your code? – Alan Birtles Mar 12 '18 at 18:45
  • 1
    Expanding on @Raindrop7 's comment: `stu = Student("John", "Smith", 1200);` is a construction of a temporary followed by assignment of the temporary, not an initialization with only a call to a constructor. – user4581301 Mar 12 '18 at 18:45
  • Compile your code with `-fsanitize=address` and you'll get useful diagnostic information. – Richard Mar 12 '18 at 19:07
  • In my get method I've tried memcpy to create a new fixed array of the same size and pass the correct element, and also just assigning something like: string abc = items[num]; return abc do these not copy the value of items[num]? – Justin Scott Mar 12 '18 at 19:16
  • `memcpy` is a C function. It is too simple to handle most C++ objects. All it does is reproduce exactly whatever you pass it, similar to the default copy and assignment operators. consider `string`. A simple `string` implementation is a wrapper around a pointer to an array of characters and some book-keeping variables like the length of the string. If you copy it stupidly, you wind up with two `string`s pointing at the same character array, and that's a disaster as soon as one of the `string`s changes its contents or is destroyed. Running out of space. – user4581301 Mar 12 '18 at 21:37
  • You need to use `string`s constructors and operators to safely copy a `string`. Only `string` knows how to handle `string`s data. `memcpy` predates C++ and has no clue that constructors exist. [`std::copy` does](http://en.cppreference.com/w/cpp/algorithm/copy), as do all of C++'s standard library containers, and the helper functions in `` and ``. – user4581301 Mar 12 '18 at 21:42

1 Answers1

2

Solution

Change

StringPtr items = new string[capacity];   

into

items = new string[capacity];

TL;DR

In

Student::Student(const string fName, const string lName, const unsigned int id)
{
    firstName = fName;
    lastName = lName;
    ID = id;

    StringPtr items = new string[capacity];
    numItems = 0;
    items[0] = "stuff";
    items[1] = "things";
    cout << items[0] << endl << items[1] << endl;
}

The line

    StringPtr items = new string[capacity];

creates a new Automatic (local) variable items and initializes it rather than the intended private member variable StringPtr items;. This is commonly known as Variable Shadowing. The local items goes out of scope at the end of the function leaking the allocated memory and the member items is never initialized leading to stu.getItem(0); accessing an uninitialized pointer and triggering the crash.

void Student::getItem(int num)
{
    cout << items[0] << endl; // items points Crom knows where, so items[0] is invalid.
}

This crash is quite fortunate. Accessing an unitintialized pointer results in Undefined Behaviour which means it could do anything from looking like it works to causing the universe to explode.

The next problem you have to deal with is observing The Rule of Three.

The best way to deal with this is std::vector

std::vector<string> items;

std::vector is Rule of Three (Rule of Five, actually) compliant so that you don't have to be.

If std::vector is not allowed, you need to implement a copy constructor

Student::Student(const Student & src):
    firstName(src.firstName), lastName(src.lastName),
    ID(src.ID), numItems(src.numItems),
    items(new string[capacity])
{
    for (int i = 0; i < src.numItems; i++)
    {
        items[i] = src.items[i];
    }
}

and an assignment operator (Taking advantage of the Copy and Swap Idiom for simplicity)

Student & Student::operator=(Student src)
{
    swap(*this,src);
    return *this;
}

Writing swap I'll leave up to you.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • So how would I ensure that the private member variable was the one initialized and filled rather than creating a new local variable? – Justin Scott Mar 12 '18 at 19:34
  • @JustinScott Point made though. I left the solution out of the answer and that makes for a pretty weak answer. – user4581301 Mar 12 '18 at 19:40