students info problem with structures c++

[valiciousx]

So I started to learn C++ and I need to solve this exercise with structures.
I need to write a program which reads students from a group then display them.The program need to be splitted in 3 modules: 1) student.h and student.cpp 2)group.h and group.cpp 3)main.cpp
Here's what I tried with some help.

  // student.h file
#ifndef _STUDENT_
#define _STUDENT_

struct Student {
	char* name;
	int mark;
};

void initStudent(Student&);
void showStudent(Student);
void delStudent(Student&);

#endif

   // student.cpp file
#include <iostream>
#include "student.h"
using namespace std;

void initStudent(Student& S)
{
	char auxN[50];
	cout << "Name: ";
	cin >> auxN;
	S.name = (char*)malloc((strlen(auxN) + 1) * sizeof(char)); // why that +1?
	strcpy_s(S.name, strlen(auxN) + 1, auxN);
	cout << "Mark:";
	cin >> S.mark;
}

void showStudent(Student S)
{
	cout << "Name: " << S.name << endl;
	cout << "Mark: " << S.mark << endl;
}

void delStudent(Student& S)
{
	free(S.name);
}

     group.h file
#ifndef _GROUP_
#define _GROUP_

#include "student.h"
struct Group {
	Student* tab;
	int nr;
	int id;
 };

void InitGroup(Group&);
void ShowGroup(Group);
void DelGroup(Group&);

#endif

// group.cpp file
#include <iostream>
#include "grupa.h"

using namespace std;

void InitGroup(Group& G)
{
	cout << "Group number: ";
	cin >> G.id;
	cout << "Students number ";
	cin >> G.nr;
	for (int i = 1, i <= G.nr, ++i)
	{
		G.tab = (Student*)malloc(G.nr * sizeof(Student));
		G.tab[i] = InitStudent(??); // each student must be initialized with InitStudent function.How to do that?
	}
}

void ShowGroup(Group G) // display the students
{
	cout << "The students are: " << endl;
	for (int i = 1; i <= G.nr, ++i)
	{
		cout << G.tab[i];
	}
}

void DelGroup(Group& G)
{
	for (int i = 1, i < G.nr, ++i)
	{
		free(G.tab[i]);
	}
}

   // main.cpp file

#include <stdio.h>
#include "student.h"
#include "grupa.h"

void main()
{
	Group g;
	InitGroup(g); // read the students
	ShowGroup(g); // display the group
	DelGroup(g); // free memory
}

The headers and main are part of the exercise.All I need to do is .cpp files.I wrote my questions in comments in the code.

[jonnin]

you are writing C / or ancient c++. Little steps, but the string variable type would be better than cstrings if you can use them. You can also add methods to structs instead of loose functions that operate on the struct type. That isnt important right now, just where you should be heading as you learn. c++ uses new and delete, malloc and free are C.


strlen gives the human length of the text. but c-strings need 1 extra slot for a special end of string char (which happens to be int value = 0, or '\0' if you prefer). so to store "hi" you need 3 spaces, because you want to store 'h', 'i', '\0' values. A string type in c++ does not need all this hands-on memory management nor extra space or any nonsense.

with your design, you need this:
G.tab[i] = InitStudent(??); // each student must be initialized with InitStudent function.How to do that?
InitStudent(G.tab[i]);


void main is not legal. main is of type int. most compilers take it, but its wrong.
<cstdio> is the c++ version. stdio.h is C.

Wherever you are learning this stuff, you are learning old or confused info, that is more C than c++, and you may be doing yourself no favors here.

[valiciousx]

Thank you for your help!I just started data structures in C++ at college.Before this I did a C course.

I get 2 errors in group.cpp.The code is:

#include <iostream>
#include "grupa.h"

using namespace std;

void initGroup(Group& G)
{
	int i;
	cout << "Group number: ";
	cin >> G.id;
	cout << "Students number ";
	cin >> G.nr;
	for (i = 1; i <= G.nr; ++i)
	{
		G.tab = (Student*)malloc(G.nr * sizeof(Student));
		initStudent(G.tab[i]);
	}
}

void showGroup(Group G) 
{
	cout << "The students are: " << endl;
	for (int i = 1; i <= G.nr; ++i)
	{
		cout << G.tab[i]; // first error
	}
}

void delGrupa(Grupa& G)
{
	for (int i = 1; i < G.nr; ++i)
	{
		free(G.tab[i]); // second error
	}
}

First error: no operator "<<" matches these operands
Second error: no suitable conversion function from "Student" to "void *" exists

What am I doing wrong?

[Ganado]

For "first error":
You are attempting to print a Student (cout << {Student object}).
But nowhere do you define what sending a Student to an output stream means.

You could do something like...

std::ostream& operator<<(std::ostream& os, const Student& student)
{
    os << S.name << " " << S.mark;
    return os;
}

For "second error":
G.tab is a Student* (pointer-to-Student).
Therefore, G.tab[i] is a Student (well, technically a Student&, but yeah).
Calling free() on a non-pointer isn't going to work.

But there's another issue: You have memory leaks. On line 15, you keep re-assigning to G.tab, overwriting the previous value.

malloc to G.tab once. Then, free(G.tab) once.


Third issue: Arrays start at index 0, not 1. Looks like you go out of bounds on the last iteration of your loops.

[jonnin]

you can make << work as above or you can print pieces, like cout << Student.name. (Or basically take the above line 3 and change os to cout and S to your variable.). the above is better, if you are ok with going up the learning curve fast. He is defining what << does when it gets your object sent to it, so it will print that way for that object now.

[valiciousx]

I got rid of the second error.I changed the line with the first error (I just removed "[i]") but after I read the students, when it needs to display them, the program crashes + I get an error from windows.
For now, the code is this:

#include <iostream>
#include "grupa.h"

using namespace std;

void initGroup(Group& G)
{
	int i;
	cout << "Group number: ";
	cin >> G.id;
	cout << "Students number ";
	cin >> G.nr;
	G.tab = (Student*)malloc(G.nr * sizeof(Student));
	for (int i = 0; i < G.nr; ++i)
	{
		initStudent(G.tab[i]);
	}
	
}

void showGroup(Group G) // display the students
{
	cout << "The students are: " << endl;
	for (int i = 0; i < G.nr; ++i)
	{
		cout << G.tab; // still not good
	}
}

void delGroup(Group& G)
{
	for (int i = 0; i < G.nr; ++i)
	{
		free(G.tab);
	}
}

I'm not familiar with that "os" and "ostream" from that solution.But I want to thank you.
I'll make some more documentation for this.

[jonnin]

#include <iostream>
I'm not familiar with that "os" and "ostream"

os is just a variable, he defined it in his code as a parameter to the function (it may not look much like any function you have seen, granted).
ostream ... you use all the time. This is what 'cout' IS.

cout << G.tab; // still not good
cout << G.tab[i]; // assuming 0 to nr is correct, this is right.

you are probably messing up a pointer somewhere, which is why c++ tends to avoid them so much that pointers being used for dynamic memory is fairly rare. They are easy to mess up and hard to fix.

[Ganado]

but after I read the students, when it needs to display them, the program crashes

Well, at least it's something. In the future, try to figure out at which point it's actually crashing in your code. This usually is a big clue as to what the problem is (although sometimes can be misleading if you have heap corruption).

You are now (correctly, as far as I can tell) setting G.tab to point to an array via malloc.

The issue is you're still free()'ing G.tab multiple times.
You should do ONE free() for every ONE malloc().

[valiciousx]

That was the problem.It works now.
Thank you for your help!