Passing character array of variable length to constructor as parameter

[dakotarosell]

Hey all, I've been working on this code for a while and can't find where I'm going wrong. We are to write a program which contains a class "Student," where each object contains a double array of grades and a character array containing the name. We are to use malloc and realloc as necessary in filling the arrays, as their length is variable. After inputting the name and the list of grades, upon receiving a grade input of "-1" the program is to print out the student's name on one line and their average grade on the next, using a function named "disp()" in the class.

I would think the average would be the more difficult part, but that part of my program works just fine. The problem is when I attempt to print the name. Before, it would print out a question mark, but after trying to solve the problem it prints nothing, almost as if it isn't storing the name in Student.name at all. If anyone could point me in the right direction, I'd really appreciate it. First post by the way, sorry if the formatting is off.

#include <iostream>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
using namespace std;

class Student
{
    private:

        double grades[];
    public:
        char name[64];
        int numgrades;
        Student(){};
        void input(const char namea[], const int lenname, double gradea[], int lengrade, int counter)
        {

            memcpy(Student::name, namea, lenname+1);


            for (int i = 0; i<lengrade; i++)
                    {
                        grades[i] = gradea[i];
                    }

                    numgrades = counter;
        }

        double mean()
        {
            double sum = 0.0;
            for (int i = 0; i<numgrades; i++)
            {
                sum += grades[i];
            }

            return sum/((double)(numgrades));
        }
        void disp();
};




void Student::disp()
        {
            cout<<"name: "<<Student::name<<endl;
            cout<<"average: "<<mean()<<endl;
        }



int main()
{
    char namelong[256];
    cout<<"Enter name: ";
    gets(namelong);
    char name1[strlen(namelong)];
    memcpy(name1, namelong, strlen(namelong)+1);


    double *gradesa;
    gradesa = (double *)malloc(sizeof(double));
    cout<<"Enter grades: \n";
    int counter = 0;
    double currgrade = 0.0;
    while (currgrade != -1.0)
    {
        double *temp = (double *)realloc(gradesa, 8*(counter+1)*sizeof(double));
        gradesa = temp;
        cin>>currgrade;
        if (currgrade != -1.0)
        {
            gradesa[counter] = currgrade;
            counter++;
        }
    }

    Student dakota;
    dakota.input(name1, strlen(namelong), gradesa, counter, counter);
    dakota.disp();

    return 0;
}

I run the program and this is what happens.

Enter name: John Doe
Enter grades:
100
50
-1
name: <---This is where the name should print.
average: 75

[Cubbi]

current gcc says

prog.cc:11:16: error: ISO C++ forbids flexible array member 'grades' [-Wpedantic]
   11 |         double grades[];
      |                ^~~~~~
prog.cc:11:16: error: flexible array member 'Student::grades' not at end of 'class Student'
prog.cc: In function 'int main()':
prog.cc:58:5: error: 'gets' was not declared in this scope; did you mean 'fgets'?
   58 |     gets(namelong);
      |     ^~~~
      |     fgets
prog.cc:59:10: error: ISO C++ forbids variable length array 'name1' [-Wvla]
   59 |     char name1[strlen(namelong)];
      |          ^~~~~

Flexible array members and variable-length arrays are C language features not found in C++ (and even in C, flexible array must be last in the struct), and gets was removed from both languages some years ago.

It's anyone's guess what happened in your program when it is compiled despite several errors. If you mention the compiler used, and if someone has the same one, they could try to see what it does.

When I use GCC old enough to have gets (specifically, gcc-4.4.7), give your "grades" a size, and allow the compiler to accept a variable-length array in a C++ program, I get the output you expect:

Enter name: John Doe
Enter grades: 
100
50
-1
name: John Doe
average: 75

but it is a really bad way to do this job in C++.. granted, the requirement "to use malloc and realloc as necessary" implies that this is a C assignment misused in a C++ course.

[dakotarosell]

Hey Cubbi, I totally agree that the assignment seems like it was built for C given the requirements. Thank you so much for your input. After reading your post and doing some troubleshooting (and a lot of trial and error), I figured out what the problem was. By not using malloc() for the name and grades values of the class, it was taking in the name properly, but then overwriting that information with the grades. For anyone who has a similar problem or might be curious about this topic, here is the corrected code for the class itself.

class Student
{
    private:

        double *grades;
    public:
        char *name;
        int numgrades;
        Student(){};
        void input(const char namea[64], const int lenname, double gradea[], int lengrade, int counter)
        {

            name = (char *)malloc(sizeof(char) * lenname);
            memcpy(name, namea, lenname);

            grades = (double *)malloc(sizeof(double) * lengrade);

            for (int i = 0; i<lengrade; i++)
                    {
                        grades[i] = gradea[i];
                    }

                    numgrades = counter;
        }

        double mean()
        {
            double sum = 0.0;
            for (int i = 0; i<numgrades; i++)
            {
                sum += grades[i];
            }

            return sum/((double)(numgrades));
        }
        void disp();
};

[doug4]

What a crappy mix of C and C++. Your instructor should be replaced.

However, since you have a class, you should have a constructor that initializes your pointers to nullptr (or NULL in the C world) and a destructor that frees anything you malloc. Right now your code has memory leaks when a Student object gets destroyed.

[dakotarosell]

Hey Doug, thanks for the heads up. I just worked through the next problem in the assignment and found similar issues and just went through some searching to figure out how to create a destructor, I'll be sure to go back and implement that in this code before I turn it in.

[keskiverto]

Rule of three/five/zero states that copy constructor&assignment need attention too:
https://en.cppreference.com/w/cpp/language/rule_of_three


That aside:

char name1[ strlen(namelong) ]; // C++ does not have VLA
memcpy( name1, namelong, strlen(namelong)+1 );

name = (char *)malloc( sizeof(char) * lenname );
memcpy( name, namea, lenname );

If we skip the VLA issue, there is still the strlen(). It does not count the terminating null.

If line 1 did allocate X bytes, then line 2 memcpy attempts to write X+1 into it.

Lines 4 and 5 do handle same amount of characters, but most likely omit the terminating null.


You do have redundant parameters. You could use a constructor in place of the input().

class Student
{
    private:
        int numgrades;
        double *grades;
        char *name;
    public:
        Student( const char* namea, double gradea[], int lengrade )
        : numgrades( lengrade ),
          grades( (double *)malloc( sizeof(double) * numgrades ) ),
          name( (char *)malloc( strlen(name)+1 ) )
        {
            if ( numgrades && grades && name ) {
              for ( int i = 0; i<numgrades; ++i )
              {
                grades[i] = gradea[i];
              }

              strcpy( name, namea );
            }
            else ; // What to do with empty Student?
        }

        ~Student();
        Student( const Student& );
        Student& operator= ( const Student& );

        double mean()
        {
            double sum = 0.0;
            for (int i = 0; i<numgrades; i++)
            {
                sum += grades[i];
            }
            // What if numgrades==0 ?
            return sum / numgrades; // sum is double, so division is double without cast
        }
};