Verbesserungsvorschlag für Studentenverwaltung

[hallihallo]

Guten Abend.

Hier die Aufgabenstellung:

Definieren sie einen Datentyp namens Student, um die Matrikelnummer, Name, E-Mail und das Geburtsdatum zu speichern. Wählen sie dazu jeweils einen passenden Datentyp. Verwenden sie ein Bitfeld, um Tag, Monat und Jahr platzsparend in einer zu speichern. Stellen sie eine Funktion zur Initialisierung von Studenten bereit, die die Membervariablen als Argumente erhält. Achten sie auch auf die Korrektheit aller Daten und Inputs. Speichern sie die Studenten zu Testzwecken in ein Array von Zeigern.

Diese Aufgabe ist dann die Grundlage der 2. Aufgabe. Dort müssen die Studenten in einer Verketteten Liste gespeichert werden.

Ok, hier mal meine .h Datei:

#ifndef STUDENT_H
#define STUDENT_H

#define MAX_LENGTH 128			/* maximum length of reg_number, forename, surename and e_mail adress */

typedef struct Birth_t
{
	unsigned int day: 5;		/* 2^5 = 32 */
	unsigned int month: 4;		/* 2^4 = 16 */
	unsigned int year: 11;		/* 2^11 = 2048 */
} Birth;

typedef struct Student_t
{
	char reg_number[MAX_LENGTH];		
	char forename[MAX_LENGTH];
	char surename[MAX_LENGTH];
	char e_mail[MAX_LENGTH];
	Birth birthday;
}Student;

/* checks, if the Birthday is valid. Returns 1, if it is valid, 0 otherwise */
int check_birthday( unsigned int day, unsigned int month, unsigned int year );

/* checks, if year is a leap year. Returns 1, if year is a leap year, 0 otherwise */
int is_leap( unsigned int year );

/* initialise student */
Student *init_student( char* reg, char* forename, char* surename, char* e_mail, unsigned int day, unsigned int month, unsigned int year );

#endif

Die Implementierungen sind trivial, vollständigkeitshalber hier die .c Datei:

#include "student.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int check_birthday( unsigned int day, unsigned int month, unsigned int year )
{
	unsigned int max_days;

	switch( month )
	{
		case 1:	case 3:	
		case 5: case 7: 
		case 8: case 10:
		case 12:	
			max_days = 31;
			break;

		case 4: case 6: 
		case 9: case 11:
			max_days = 30;
			break;

		case 2:		
			max_days = 28 + is_leap( year );
			break;

		default:
			printf( "Wrong month. Only months between 1 ( January ) and 12 ( December ) are allowed. Your input was %u\n", month );
			return 0;
			break;
	}

	if( day > 0 && day <= max_days )
		return 1;
	else
	{
		printf( "Wrong day. Only days between 1 and %u are allowed. Your input was %u\n", max_days, day );
		return 0;
	}
}

int is_leap( unsigned int year )
{
	if( ( year % 4 == 0 && year % 100 != 0 ) || year % 400 == 0 )
		return 1;

	return 0;
}

Student *init_student( char* reg, char* forename, char* surename, char* e_mail, unsigned int day, unsigned int month, unsigned int year )
{
	Student *student = malloc( sizeof( Student ) );
	if( student == NULL )
	{
		printf( "malloc fails" );
		exit( EXIT_FAILURE );
	}
	strcpy( student->reg_number, reg );
	strcpy( student->forename, forename );
	strcpy( student->surename, surename );
	strcpy( student->e_mail, e_mail );
	student->birthday.day = day;
	student->birthday.month = month;
	student->birthday.year = year;
	return student;
}

und hier die main Datei:

#include "3_Schweigl_Aufgabe1_student.h"
#include <stdio.h>
#include <stdlib.h>

#define MAX_STUDENTS 50
#define MAX_INPUT 1024

int main()
{
	Student *students[MAX_STUDENTS];
	int chose = 1;
	int student_counter = 0;
	do
	{
		printf( "\n\nChose one of the following options\n" );
		printf( "Initialise a student (1)\n" );
		printf( "Show all students (2)\n" );
		printf( "Quit (0)\n" );

		scanf( "%d", &chose );
		getchar();
		switch( chose )
		{
			case 1:
			{
				if( student_counter < MAX_STUDENTS )
				{
					char temp_reg_number[MAX_LENGTH];
					char temp_forename[MAX_LENGTH];
					char temp_surname[MAX_LENGTH];
					char temp_e_mail[MAX_LENGTH];
					unsigned int temp_year;
					unsigned int temp_day;
					unsigned int temp_month;
					char buffer[MAX_INPUT];
					printf( "Please enter registration number, forename, surname, e-mail, birthday, month and year each separated by a whitespace\n" );
					printf( "Example: 123456789 Max Mustermann Max@Mustermann.com 12 12 2012\n" );
					fgets( buffer, MAX_INPUT, stdin );
					if( sscanf( buffer, "%s %s %s %s %u %u %u", temp_reg_number, temp_forename, temp_surname, temp_e_mail, &temp_day, &temp_month, &temp_year ) != 7 )
					{
						printf( "Invalid input. Try it again\n" );
						continue;
					}
					else
					{
						if( check_birthday( temp_day, temp_month, temp_year ) )
							students[student_counter++] = init_student( temp_reg_number, temp_forename, temp_surname, 
																		temp_e_mail, temp_day, temp_month, temp_year );
						else
						{
							printf( "invalid birthday. Try it again\n" );
							continue;
						}
					}
				}
				else
				{
					printf( "You can not insert more than 50 students at the same time\n" );
					continue;
				}
			}
			break;

			case 2:
				for( int i = 0; i < student_counter; ++i )
					printf(" %s %s %s %s %u %u %u", students[i]->reg_number, students[i]->forename, students[i]->surename, students[i]->e_mail, 
													students[i]->birthday.day, students[i]->birthday.month, students[i]->birthday.year );
					break;

			case 0:
				break;

			default:
				printf( "You have to chose one of the options above.\n" ); 
				break;
		}
	}while( chose != 0 );

	return EXIT_SUCCESS;
}

Der Code funktioniert. Er fängt Datumsfehler und Inputfehler ab. Aber ich finde den Code ehrlich gesagt nicht schön, darum wollte ich euch fragen, ob ihr Verbesserungsvorschläge habt. Vor allem Zeile 28 - 34 in der main Datei gefallen mir nicht. Ich weiß aber auch nicht, wie ich ohne temporäre Variablen auskommen soll.

Ich bedanke mich schon mal und wünsch euch ein schönes Wochenende

[DirkB]

hallihallo schrieb:

Vor allem Zeile 28 - 34 in der main Datei gefallen mir nicht. Ich weiß aber auch nicht, wie ich ohne temporäre Variablen auskommen soll.

Bei dir ist das relativ einfach, da deine struct Student_t schon alles enthält (Du hast echte Arrays mit viel Platz).

Also reicht da doch ein

Student student;

Allerdings wirst du später alle deine char-Arrays durch Zeiger ersetzen und den Speicher dafür auch dydnamisch anfordern. (spart Speicher).
Daher kommst du um die temporären Variablen nicht herum.

Deine struct Birth_t ist doch eher eine struct Date_t , da man sie auch für andere Angaben nutzen kann (Im- Und Ex-matrikulationsdatum, ..). Jahresangaben bis 2048 ist ein bisschen kurz gegriffen.
Vor allem hast du ja gerade mal 20 Bit belegt. Das sind entweder 4 zuviel oder 12 verschwendet

Bei check_birthday legst du ein 2D-Array an.

int check_date( unsigned int day, unsigned int month, unsigned int year )
{ static int days_per_month[][13] = { { 0, 31, 28, 31, ... , 31},
                                      { 0, 31, 29, 31, ... , 31} };

    unsigned int max_days;

    max_days = days_per_month[is_leap( year )][month];

    if( day > 0 && day <= max_days ) ....

Du Funktion init_student gibt doch bitte auch NULL zurück wenn kein Speicher mehr da ist und die aufrufende Funktion macht eine Meldung. Dann kannst du immer noch dein Programm nutzen und zb. Die Daten speichern oder löschen.

Beim sscanf solltest du noch die Größe der Arrays berücksichtigen.

sscanf( buffer, "%127s %127s %127s %127s %u %u %u",

Beim printf fehlt das '\n'

Bei der Schwester vom Max, die Marie Sophie Mustermann, hast du auch Spaß mit deinem Programm.
Leerzeichen sind bei solchen Feldern als Trenner nicht geeignet.

[hallihallo]

Hallo DirkB.

Bei dir ist das relativ einfach, da deine struct Student_t schon alles enthält (Du hast echte Arrays mit viel Platz).
Allerdings wirst du später alle deine char-Arrays durch Zeiger ersetzen und den Speicher dafür auch dydnamisch anfordern. (spart Speicher).
Daher kommst du um die temporären Variablen nicht herum.

Also soll ich die Membervariablen reg_number, forename, surname und e-mail zu Zeiger machen und dann den speicher in init_student mittels malloc allokieren?

Deine struct Birth_t ist doch eher eine struct Date_t, da man sie auch für andere Angaben nutzen kann (Im- Und Ex-matrikulationsdatum, ..). Jahresangaben bis 2048 ist ein bisschen kurz gegriffen.

Ok, das klingt logisch. Werde ich gleich ändern.

Vor allem hast du ja gerade mal 20 Bit belegt. Das sind entweder 4 zuviel oder 12 verschwendet

Ja, genau den gleichen Gedanken hatte ich auch schon. Mit weniger komme ich auf keinen Fall aus. Aber 12 Bits mehr fürs Jahr wäre schon etwas zu viel. Naja, bevor ich sie verschwende verwende ich sie lieber
Aber danke für den Denkanstoss. Werde das als Kommentar im Programm einfügen.

Bei check_birthday legst du ein 2D-Array an.

Das gefällt mir.

Du Funktion init_student gibt doch bitte auch NULL zurück wenn kein Speicher mehr da ist und die aufrufende Funktion macht eine Meldung.

Stimmt, das wäre sinnvoll.
Soll man eigentlich Fehler immer so behandeln?

if( do_some_stuff() )
printf( "OK" );
else
printf( "Fehler" );

Also den Rückgabewert checken?
Soll ich dann in check_birthday( ... ) das printf weglassen und den Fehler erst außerhalb durch den Rückgabewert melden?

Bei der Schwester vom Max, die Marie Sophie Mustermann, hast du auch Spaß mit deinem Programm.
Leerzeichen sind bei solchen Feldern als Trenner nicht geeignet.

Das ist auch so ein Punkt, der mir nicht gefällt. Aber ich wusste nicht, wie ich es sonst machen sollte. Mein Problem ist hier die E-Mail Adresse, da sie sehr viele Sonderzeichen unterstützt. E-Mail Adresse
Wie würdest du herangehen?

Vielen Dank für deine Antworten. Die haben mir sehr geholfen.

[DirkB]

Wie du Fehler behandelst hängt von der Umgebung ab. Bei der Gui ist es etwas anders als bei einem Konsolenprogramm.
Funktionen wie init_student müssen aber gar nicht mit dem Anwender kommunizieren.

Auch ist es wichtig, den Fehler nicht nur zu melden, du musst auch darauf reagieren.
Von daher ist das exit( EXIT_FAILURE ); schon mal gut.
Viele melden zwar den Fehler, machen aber dann so weiter als ob nichts wäre.

Genauso bei check_birthday. Das prüft nur. Die Reaktion auf eine Fehleingabe macht die rufende Funktion. Bzw deren rufende ....

hallihallo schrieb:

Aber 12 Bits mehr fürs Jahr wäre schon etwas zu viel.

Mit einem Bit mehr kommst du schon bis 4096. Das reicht doch schon.

hallihallo schrieb:

Also soll ich die Membervariablen reg_number, forename, surname und e-mail zu Zeiger machen und dann den speicher in init_student mittels malloc allokieren?

Besser ist das. Muss nicht sofort sein. Behalte das im Hinterkopf.

[hallihallo]

Danke DirkB. Hat mir wirklich sehr geholfen