Proszę o sprawdzenie programu

Cześć,

napisałem sobie dziś projekt do szkoły na PRI i chciałbym poprosić kogoś doświadczonego, aby na to zerknął. Oto treść zadania:

Należy stworzyć spis uczniów liceum. Program powinien umożliwiać
wczytanie do N = 40pozycji zawierających:

• nazwisko

• imię

• klasę (od 1 do 3 np. 2b lub 3a)

• średnią z poprzedniego półrocza (np. 3.56)

Listę należy wyświetlić:

a)wg alfabetycznej kolejności nazwisk i imion

b)wg średniej (rosnaco), a dla takiej samej średniej wg klas
(alfabetycznie i rosnaco, np. 1a 2a 3b)

Wyświetlone pozycje powinny zawierać pełną informację, tzn. nazwisko,
imię, średnią i klasę. Program powinien umożliwiać edycję istniejących i
dopisywanie nowych pozycji po wyświetleniu listy, reagować i być odporny
na nielogiczne wprowadzanie danych (np. klasa: Abc).

Kod: http://4programmers.net/Pastebin/1113

Chodzi mi głównie o sprawdzenie czy np. można jeszcze coś zoptymalizować (czasowo/ pamięciowo lub składniowo - żeby kod był krotszy). Mam jeszcze sporo rzeczy do zrobienia, a niestety sam rzadko kiedy potrafię poprawiać swoje błędy ;/

Twój kod jest zbyt ściśnięty. Strasznie ciężko się go czyta. W wielu miejscach piszesz jakiś magiczny kod, który nie wiadomo co robi

if(uczen->imie[0] < 65 || uczen->imie[0] > 90)

Po drugie taki zapis jest podatny na błędy.
funkcja swap: wiesz, że można przypisywać strukturę do struktury?
Rozłóż kod na większa liczbę funkcji, ewentualnie stosuj komentarze - ale pierwszy sposób lepszy.

while(poprawne || !poprawne)

?

błędów oczywiście jest znacznie więcej, ale ciężko się tak analizuje.

Odnośnie pierwszego fragmentu kodu - sprawdzam czy pierwsza litera imienia jest duża.

Drugi fragment kodu - chciałem uzyskać efekt pętli nieskończonej. Niestety zwykle while(true) nie podziałało.

à propos swapa - ale jak ma się przypisywanie struktury do struktury, do przenoszenia danych w obrębie jednej struktury?

Poza tym, właśnie chodzi mi o wskazanie miejsc, które dla osoby próbującej analizować kod, wydają się zawile i niezrozumiałe. Ja niestety, z tego względu, że to pisałem, wszystko rozumiem i wydaje mi się jasne.

Postarałem się trochę rozjaśnić kod i dodałem komentarze - http://4programmers.net/Pastebin/1114

Odnośnie pierwszego fragmentu kodu - "Łukasz" odpada
Drugi fragment kodu - while(true) lub for(;;)
swap - przypisuje składową po składowej

Owszem wszystko jest jasne ale zbyt zamulone bezsensownymi rozwiązaniami.

1. Czemu nie akceptujesz polskich znaków w imieniu i nazwisku?
2. Zdarzają się podwójne nazwiska, czemu nie uwzględniasz?
3. Czemu nie napiszesz funkcji kwalifikującej znak, w tym polskie litery?
4. Czemu program wymaga wpisania imienia z dużej litery zamiast zamienić małą na dużą, w czym problem?
5. Tak strasznie pilnujesz imienia i nazwiska zaś klasę można wpisać np: 0x
6. Trzymanie średniej jako napisu kompletnie bezsensowne, a jak ci ktoś każe rozszerzyć program o liczenie średniej dla klasy? dla roku? dla szkoły?
7. Co będzie jak jakieś nazwisko nie zmieści się w wyznaczonych rozmiarach? Czemu nie zrobiłeś dynamicznie?

Właściwie samo wyliczenie wszystkich wad twojego kodu zajęło by prawie tyle samo miejsca na dysku co twój kod.

No zabrakło mi cierpliwości przy pisaniu tego właśnie na takie rzeczy :P Pytanie tylko - jak sprawdzić czy coś, co zostało wpisane podczas prośby o średnią, faktycznie znajduje się w zakresie <1.00;6.00>

Przy

if(srednia >= 1.00 && srednia <= 6.00)

Przechodzi mi test dla np. 6..2, ale przy wypisywaniu pokazuje głupoty. Dlatego dałem to jako char, żeby wszystko sprawdzać znak po znaku

Po pierwsze raczej powinieneś sprawdzać od 3.00 do 6.00 bo jak ma średnią poniżej 3.00 to znaczy że z przynajmniej jednego przedmiotu ma mniej niż 3.0 a to znaczy że nie mógł z taką oceną przejść do następnego roku.
Po drugie:

double d;
if(scanf("%lf",&d)==1)
  {
   if((3<=d)&&(d<=6)) printf("ok\n");
   printf("poza przedziałem\n");
  }
else printf("a to nie liczba\n");

To, że ze średnią poniżej 3.0 się nie zdaje, to nie oznacza, że nie można mieć jakiejkolwiek, poniżej tej. Nie wnikamy w programie, czy ktoś zdał czy nie tylko jaką miał średnią.

a po 2 - już tak zrobiłem i np. właśnie dla 4..2 scanf zwraca 1 :) i przechodzi test

To wpisałeś 4..2 zaś wczytało się 4. (czyli 4 równo) zaś .2 pozostało w buforze klawiatury, możesz po scanf() sprawdzić czy kolejne znaki przed \n nie są spacjami i tabulacjami to znaczy że użytkownik wpisał coś ekstra.
Zauważ że jak wpiszesz "4.3ala ma kota" to ci też to przyjmie.

Dobra, z tym to już sobie dam radę. Teraz mam inny problem, równie banalny ale mam jakąś blokadę we łbie.

typedef struct{
    char* nazwisko;
    char* imie;
    char klasa[2];
    double srednia;
} uczen;

Deklaracja metody do wczytywania danych ucznia:

void wczytajDane(uczen *uczen)

Wywołanie:

wczytajDane(&listaUczniow[aktualnyUczen]);

I nie wiem jak w metodzie wczytajDane, wczytac poprawnie imie i nazwisko ucznia. Dopóki te zmienne były statyczne, czyli char nazwisko[40] i char imie[20], wystarczyło

scanf("%s", &uczen->imie). Gdy deklaracja wyglada tak jak powyzej, program mi sie wysypuje po wpisaniu imienia.

A alokujesz gdzieś pamięć na to imie, czy nazwisko?

uczen->imie = (char*)malloc(sizeof(char)*ileśtam);

Dobra, działa. Zrobilem tak jak pisałeś Shalom już wcześniej, ale przy alokacji pamięci zamiast strlen(&dane)*sizeof(char) wpisałem strlen(dane)*sizeof(char) i się chrzaniło. (char *dane)

Napisz sobie coś co wczytuje wiersz nieograniczonej długości, coś w stylu:

char *wczytaj_wiersz()
  {
   char buf[16],*tmp,*ret=NULL,more=1;
   int len=0,add;

   while(more)
     {
      fgets(buf,sizeof(buf),stdin);
      add=strlen(buf);
      if(buf[add-1]=='\n') buf[--add]=more=0;
      tmp=(char*)malloc(len+add+1);
      if(len)
        {
         memcpy(tmp,ret,len);
         free(ret);
        }
      ret=tmp;
      memcpy(ret+len,buf,add+1);
      len+=add;
     }
   return ret;
  }

użycie:

uczen->nazwisko=wczytaj_wiersz();

z tym że musisz pamiętać jak już ten uczeń nie jest potrzebny to trzeba wywołać:

free(uczen->nazwisko);

Cześć,
radził bym napisać to obiektowo, w klasach z polami i metodami.
Kod będzie o wiele bardziej intuicyjny, przejrzysty i modyfikowalny!
Pozdrawiam

@up jak byś nie widział kod jest pisany w C gdzie nie istnieje pojęcie klasa.

To jest jakaś porażka. Nie umiem najprostszych rzeczy na wskaźnikach O_O

void wczytajDane(uczen *Uczen, int czyEdycja)
{
    int poprawne=1, i, dots = 0;
    char *dane, srednia[5];
    if(czyEdycja)
        printf("Aktualne imie ucznia: %s\n Nowe imie:", Uczen->imie);
    else
        printf("Podaj imie ucznia: ");
    scanf("%s", &dane);
    printf("%d", &Uczen);
    Uczen->imie = (char*) malloc(strlen(&dane)*sizeof(char));
    printf("COS!");
    strcpy(Uczen->imie,&dane);

to jest fragment funkcji. Właśnie tutaj alokuję pamięć dla obiektu, który jest w tablicy, tworzonej w mainie. Wywolywana jest w ten sposób:

wczytajDane(&listaUczniow[aktualnyUczen],0);

Problem polega na tym, że przy każdym wywołaniu tej funkcji (DLA RÓŻNYCH UCZNIÓW!) adres pamięci Uczen jest taki sam, przez co nie mogę zaalokować pamięci dla następnego obiektu.

Nie możesz zrobić:

char *dane;
scanf("%s",dane); // tu mażesz po pamięci

można zaś:

char buf[513];
scanf("%512s",buf); // bardzo ważne %512s użytkownik nie będzie mógł wpisać więcej, nie będzie mazania po pamięci.
uczen->imie=strdup(buf); // strdup oblicza długość parametru, przydziela pamięć, kopiuje do niej parametr.

Lub jednak użyj funkcji którą podałem wyżej, od razu zwraca przydzieloną pamięć.

No to równie dobrze, mogę sobie nazwisko i imie zakeklarować jako char[512] i nie bedę się musiał w to bawić :D poza tym, to i tak się nie wywala na wczytywaniu, tylko alokacji pamięci, więc takie rozwiazanie nie naprawi błędu. Ten printf, ktory widzicie w kodzie - wstawiony przed mallociem wypisuje się, po już nie. Więc wiemy gdzie jest błąd.

Nie ważne gdzie się wywala, ważne gdzie jest pierwszy błąd. A błąd jest tu:

char *dane;
scanf("%s", &dane);

Po pierwsze dane nie są przydzielone.
Po drugie niepoprawne wywołanie scanf, ma być:
scanf("%s",dane);
ponieważ zmienna dane już jest wskaźnikiem na obszar.
Przydzielić jakiś obszar pamięci musisz przed scanf'em.
Przydzielenie pamięci po scanf to jakby już po ptakach.

Zapis

scanf("%512s",buf);

Nie działa. Bez tego 512 idzie bez problemu, a z tym wpisuje mi jakieś krzaczki do zmiennej.

Krzaczki ci w innym miejscu wyskakują:

#include <stdio.h>

int main()
  {
   char buf[513];
   printf("inp: ");
   scanf("%512s",buf);
   printf("out: %s",buf);
   fflush(stdin);
   getchar();
   return 0;
  }

Masz w tym kodzie krzaczki?