I need help with my C++ program

Development Code
62

It is indeed for school. The full code is hosted at GitHub - LinuxDragon57/ClassRoster-WGU: This is the program for C867 at WGU.. I’ve tried using delete, but there’s still a segmentation fault as soon as the program exectues that line of code. At first, that line read, delete classRosterArray[--classRosterSize];, for I was just performing two operations with one line of code unlike now.

I do need to get rid of the student altogether, though, because later on I am going to have to call a printAll method. You can read main to see what the program does - spoiler, it’s a worthless program of insanity just cause school can make it that way. Honestly I didn’t know what {} does. I just saw that someone else used it in their code and I thought I’d try it since delete wasn’t working for me. I figured I’d figure out what it did when I tried to use it, but I was still working on that when you told me xD.

63

ah jeez, this is driving me nuts… :slight_smile: I can’t spot it.

Clearly the default destructor is trying to get rid of something, and it’s not going well, but what?

I thought since Student.h says: string, int, int*, DegreeProgram

It could be this int* which is basically a raw array.

In Roster::add you’re defining int daysInCourseArray[3] on stack, and it decays to a pointer when passed to a constructor, and I thought that looks weird, does this pointer to an array in the stack get saved in Students which then outlives the roster::add stack, …but no… then I saw you’re copying the raw array pointer once again to setDaysInCourse which then copies the elements using a for loop (instead of e.g. std::copy) … but it still looks weird. … any chance you can try adding/removing a Student without ever calling setDaysInCourse from a constructor.

(raw pointers and raw arrays and new/delete grumble grumble … we have shared_ptr/unique_ptr and std::vector for a reason, it saves so much sanity)

ramblings -it's all fishy

delete tries to deallocate memory behind a pointer. But more importantly, it calls destructors before “freeing” memory.

(freeing usually just means giving it back to some standard library, so that other parts of the app can reuse when they need it).

delete *this->classRosterArray; looks a bit fishy.

1 Like
64

Yeah, I am pretty sure that it is a scoping issue somewhere because of the way that the points are going through a mix of copies, and passes. I also zeroed in on some of the items that you also mention above but, I have only been looking at this during some free time at work. I had made some changes the student and roster classes using some more sane mechanisms that did not adhere to the requirements document and got things to work; but that does no good for the actual assignment.

Unfortunately, I am running this with the MS Windows C++ compiler since I could not get MingW to spit out fully compatible win32 binaries; well sometimes they would partially work and then just hang with no crash and no errors. I am not necessarily a fan of the Microsoft compiler or their debugger.

65

I’m probably going to have to test this program using Microsoft’s compiler cause it’s so different than the GNU C++ compiler.

66

From what I understand, there’s something wrong with the strings. I’ll definitely take your advice about the raw array issue, as I have had the hardest time getting the array passed around to functions in the memory. I know that this program is insane. I literally had to ask my professor what one part of the instructions meant. When I started this program I was so mad that it told me how to write the program without actually telling me what I am writing. But after I started writing roster.cpp, I realized that it never tells me what I am writing, because I am writing nothing. This stupid piece of shit has no real world, practical application.

67

I have to use the mutator as per the instructions. At any rate, what should I change about the code to fix it? Please don’t do a merge request because, with it being for school, they might think I was cheating which I am not as it’s obvious that I am not the only one perplexed by the remove function’s wierd behavior. (Also, for the destructor, I literally copied someone else’s function because I didn’t even know where to begin with implementing a destructor. I am not an OOP-minded person and I honestly prefer anything but OOP because OOP adds unnecessary complexity).

68

Here is a working minimal example of what you are trying to accomplish but with std::strings (or, really, any sufficiently advanced class). Hope it helps.

The code gives the following output:

array: size 3
    Hello
    My
    Friend!
array: size 1
    Hello
69

Yeah, but the remove function looks like it doesn’t do what it needs to do. You have to pass in a parameter to the remove function. That paramter tells the function which element from the array to remove using the this->getByID() function. Removing the last two elements from the array doesn’t actually do what it’s supposed to do.

70

@wertigon printArray() doesn’t do it’s job correctly either. Idk what you are trying to say is wrong with my function, but your code doesn’t help me understand what I am doing wrong. Currently my function printAll() looks like this:

for (auto& index : classRosterArray) index->print();

where the print method called in that function looks like this:

    cout << this->getStudentID() << " \t";
    cout << "First Name: " << this->getFirstName() << " \t";
    cout << "Last Name: " << this->getLastName() << " \t";
    cout << "Age: " << this->getAge() << " \t";
    cout << "daysInCourse: {";
    for (int i=0; i<3; i++) cout << this->getDaysInCourse(i) << ',';
    cout <<"} \t";
    cout << "Degree Program: ";
    switch(this->getDegreeProgram())
    {
        case 0:
            cout << "Security" << endl;
            break;
        case 1:
            cout << "Networking" << endl;
            break;
        case 2:
            cout << "Software" << endl;
            break;
        default:
            cout << "Unspecified" << endl;
    }
71

Also, guys, my professor saw my program on Github and recommended that I take it off - probably at least until after I submit it. I didn’t do anything wrong, but I think I understand why he said that. When I submit it, the plagarism checker will see my github-hosted code and think that I plagarized from that because it doesn’t know that my github account is mine…

Edit: All I did was make the repository private. Hopefully that’s enough to fool the plagarism detection system.

72

Also, Idk if you think of yourself as smarter than me, but I know the difference between the pre-increment and post-increment in C/C++.

73

Oh, yeah, I never said it was complete, just that the example should help you at the parts you are struggling at for better understanding.

I am aware the print function is not what you want, but if you want an easy way to print an object in the future, I do suggest you look into ofstream operator overloading (think of it as the equivalent of __str__ in Python). That is not relevant for this problem however, but would probably give brownie points. :slight_smile:

A remove function that takes parameters is slightly worse to implement with arrays, but not very, instead of:

void remove() {
    if (this->count > 0) {
        delete this->coreArray[this->count];
        this->coreArray[this->count--] = 0;
    }
}

Do

void remove(int key) {
    for (char i=0; i < this->count; ++i) {
        if (has_key(key, i)) {    // has_key compares key with indexed value
            this->count--;
            swap(this->coreArray[i],this->coreArray[this->count]);
            delete this->coreArray[this->count];
            return;
        }
    }
}

Note that I have only shown you how to think about adding and deleting; in the real world this code does not communicate whether something could be deleted, or not.

Incidentally, to get a class working with the template container I posted above, you’d need to get the copy constructor and ofstream operator working. Other than that I think it should work great for any object. :slight_smile:

74

All the functions seem to be working. My problems seem to be centered around the remove function. The add function does work. I know that for sure, and everything else seems to be in working order. Also when you said, “add by copy constructor - really clever, but very obscure,” what did you mean by that?

75

If you come from other languages you’ve probably never used a copy constructor, but what it does, compare:

MyClass object();
MyClass *mcPtr = new MyClass(object);

With

MyClass object();
MyClass *mcPtr = new MyClass();
*mcPtr = object;

Two ways to do the same code, really (if we assume = and copy constructor both perform a deep copy). Most tend to go with the latter option to make code more readable.

Same thing with myArr[count++] - this is no different from myArr[count]; count++; yet the latter is often much, MUCH more readable = easier to locate bugs.

Often, to gain understanding it is important to take a step back and then one step forward. Perhaps even rewrite the function a second time just for claritys sake.

2 Likes
76

But why shouldn’t my method of doing this work?

void remove(int deleteIndex) {
    for (; deleteIndex < this->rosterIndex; deleteIndex++) {
        this->classRosterArray[deleteIndex] = this->classRosterArray[deleteIndex+1];
    }
    delete this->classRosterArray[this->rosterIndex--];
}

This may not be exact, but I mean to show the logic. That is, I am just starting at the index I want to delete and overwriting its information with the information from the next element in the array. Then I can just delete the last element in the array.

77

Ok, so let’s just map this one out.

Let’s assume we have an array of four names (of five):

arr = { "Tom", "Dick", "Harry", "John", 0 }

We want to remove the third name:

arr = { "Tom", "Dick", 0, "John", 0 }

This is the array state for each of your state:

dI = 2                             /* Delete index */
rI = 4                             /* Roster index */
move(arr, dI+1, dI)                /* Move dI+1 to dI */
dI++
move(arr, dI+1, dI)                /* Move dI+1 to dI */
dI++
delete(arr, rI)                    /* Remove array element rI */
rI--

Which gives the following motions:

arr = { "Tom", "Dick", "Harry", "John", 0 }    /* dI=2, rI = 4 */
arr = { "Tom", "Harry", "Harry", "John", 0 }   /* move(), dI = 3 */
arr = { "Tom", "Harry", "John", "John", 0 }    /* move(), dI = 4 */
arr = { "Tom", "Harry", "John", "John", 0 }    /* delete(), segfault */

Why? Because you try to delete element 4 (e.g. the fifth element) in an array with index 0-4. This segment is 0. :slight_smile:

Also, by swapping index 2 and 3 and deleting 3 you have an algorithm with O(1) moves instead of O(n) moves, like in your case. The algorithm seems to work otherwise.

2 Likes
78

Yeah the problem is indeed coming from when the roster::printAll() method goes out of bounds of the array because the value there doesn’t exist. So I need to figure out how to make the array 1 element smaller.

79

Or simply abort if the array points to a null pointer? That is,

for (auto& index : classRosterArray) {
    if (index == 0) {
       return;
    }
    index->print();
}
1 Like
80

So I’ve built a copy constructor,

Roster::Roster(const Roster& self)
{
    for (auto& index : this->classRosterArray)
    {
        index = new Student();
        index = *self.classRosterArray;
    }
    this->rosterIndex = self.rosterIndex;
}

I don’t know if this is what I am supposed to do, but I tried. I don’t even know what its purpose is. Gosh I hate C++. It should be called C-- because it’s worse than C. Also what does my move Constructor need to do?

I used Copy Constructors and Copy Assignment Operators (C++) | Microsoft Docs to help me understand Copy Constructors and Move Constructors.

P.S I am using that same resource to try and implement my move constructor.

81

Wait I don’t think that’s going to work. This program forces too much OOP on people. Classes do nothing except create more code surface area for which more bugs can pop up. F**k this stupid program.

If I had my way I’d be doing this program way differently because modern OOP creates bad coding habits, but I have to do this because the school wants me to learn this way. Ooh I hate OOP so much.