Sprawdzenie programów

0

Mógłby ktoś sprawdzić te programy? W sensie czy nie ma żadnych niepotrzebnych linijek czy można byłoby to napisać lepiej?

#include <stdio.h>
#include <stdlib.h>
#include <math.h>




main(int argc, char *argv[])
{ 
float a,b,wynik; 
char *c=argv[2]; 
if (argc!=4) 
{ printf("\nLiczba argumentow rozna od 4!\n"); 
 }  else{
a=atof(argv[1]);  
b=atof(argv[3]);  
 
switch (*c){    
       case '+' :  wynik=a+b; 
       printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik); 
       break;    
       case '-' :  wynik=a-b; 
       printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik); 
       break;     
       case 'x' :  wynik=a*b;
       printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik); 
        break;     
       case '/' :if(b==0) printf("nie mozna dzielic przez 0");
       else {wynik=a/b; 
       printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik); } 
       break; 
       case 'p' :  if(b<0 || a<0) printf("a musi byc wieksze lub rowne 0, b musi byc wieksze lub rowne 0");
       else {wynik=pow(a,1.0/b); 
       printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik); }
       break;    
       default : printf("Program wykonuje obliczenia (+,-,x,/,a p n(to jest pierwiastek z a stopnia n)) dzialanie nalezy wpisac przy wywolaniu programu w postaci np. a + b"); 
       }     
 

}
return 0;
}
 
#include<stdio.h>
#include<stdlib.h>
#include<math.h>

struct point
{
  float x;
  float y;
};

float odl (struct point a, struct point b);

int main()
{

struct point a, b;
 printf("podaj wspolrzedne punktu a \n");
 scanf("%f %f", &a.x, &a.y);
 printf("podaj wspolrzedne punktu b\n");
 scanf("%f %f", &b.x, &b.y);
 printf("punkt a ma wspolrzedne (%.2f, %.2f)\n", a.x, a.y);
 printf("punkt b ma wspolrzedne (%.2f, %.2f)\n", b.x, b.y);
 printf("odleglosc miedzy a i b to %.2f\n", odl(a,b));

return 0;
}

float odl (struct point a, struct point b)
{
 return sqrt((a.x-b.x)*(a.x-b.x)+(a.y-b.y)*(a.y-b.y));
}
 

programy niby działają ale informatyk mi nie chce tego zaliczyć i nie wiem dlaczego...

0

W pierwszym:

main(int argc, char *argv[])

Brak "int"

0

Przede wszystkim brzydkie wcięcia. Popraw je

3

Odniosę się do pierwszego programu.

Porządne sformatowanie kodu to teraz najważniejsze zadanie. Kod ma być elegancko wcięty. W jednej linijce ma być jedna instrukcja. Bez cyrków typu:

       case 'p' :  if(b<0 || a<0) printf("a musi byc wieksze lub rowne 0, b musi byc wieksze lub rowne 0");
       else {wynik=pow(a,1.0/b); 
       printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik); }
       break; 

Powyższy kod sformatuj inaczej:

       case 'p' :
         if(b < 0 || a < 0)
           printf("a musi byc wieksze lub rowne 0, b musi byc wieksze lub rowne 0");
         else {
           wynik = pow(a, 1.0 / b); 
           printf("\n %0.3f %s %0.3f = %0.3f \n", a, c, b, wynik);
         }
         break; 

Nie sądzisz, że tak jest znacznie czytelniej i jaśniej? A zmieniłem tylko układ spacji i znaków nowej linii. Osobiście dodałbym jeszcze klamry { oraz } wokół instrukcji po ifie, ale to już szczegół.

Innym problemem są powtórzenia. Eliminowanie powtórzeń to jedna z podstawowych zasad programowania (zobacz: http://pl.wikipedia.org/wiki/DRY ).

Pomyśl: co jeśli nagle zdecydujesz, że zamiast trzech cyfr po przecinku chcesz wyświetlać dwie -- albo cztery? Będziesz musiał zmodyfikować powtórzony kod w kilku miejscach i mieć nadzieję, że żadnego nie przeoczyłeś.

Powtarza się u Ciebie instrukcja wyświetlająca wynik:

printf("\n %0.3f %s %0.3f = %0.3f \n",a,c,b,wynik);

Powinieneś wywalić ją z poszczególnych case-ów i przenieść za switch-a. Ale, ale, nie możesz tego zrobić tak łatwo, bo w trzech przypadkach nie wykonujesz działania i nie wyświetlasz wyniku, tylko coś innego. Tymi wyjątkowymi przypadkami są: dzielenie przez zero, potęgowanie poza zakresem i wyświetlenie pomocy. Zrób więc tak...

Przed switchem utwórz sobie zmienną int wyswietl_dzialanie = 1;. Domyślnie ustawiamy ją na 1 (TRUE, prawda), co będzie oznaczało, że za switchem wyświetlimy działanie wraz z wynikiem. W tych trzech przypadkach, w których działania nie wykonujemy, wstaw po prostu przypisanie wyswietl_dzialanie = 0;

Usuń (wytnij) te powtarzające się instrukcje printf. Dodaj jedną taką instrukcję, w jednym miejscu -- za switchem. I niech się wykonuje tylko jeśli wyswietl_dzialanie ma wartość prawdziwą (tj, różną od zera), czyli tak:

if (wyswietl_dzialanie) {
  printf("\n %0.3f %s %0.3f = %0.3f \n", a, c, b, wynik);
}
0

bswierczynski ma dużo racji, ale nie zgodze sie z nim w przypadku powtarzającego się kodu, tzn trzeba unikać powtórzeń ale nie w taki sposob. Nie ma to jak znienacka pojawiajaca sie zmienna logiczna ktora jest potem sprawdzana gdzies tam w kodzie. teraz wyobraź sobie że masz wiecej takich miejsc i cały czas dopisujesz nowe zmienne ktore maja tylko sprawdzac czy jakis kod ma się wykonac czy nie. Nie po to ktoś używa switcha żeby potem jeszcze raz sprawdzać warunki logiczne. Nie chcę krytykować za bardzo wypowiedzi bswierczynskiego ale czasem lepiej napisać trochę wiecej kodu żeby był czytelny i żeby nie trzeba było szukać programu między stertą niepotrzebnych zmiennych.

1

@a:
A ja podtrzymam jednak adekwatność użycia tutaj zmiennej znacznikowej. Nie proponuję, by wprowadził do procedury "więcej" takich zmiennych -- tylko tę jedną. Zauważ, że z 3 przypadków wyjątkowych 2 i tak nie są obsługiwane przez samego switcha, tylko zwykłe instrukcje warunkowe w poszczególnych przypadkach (case'ach). Więc to nie tak, że sprawdzamy tu pewne warunki "jeszcze raz".

DRY to bardzo ważna, priorytetowa zasada. Eliminacja pięciu (!) powtórzeń kosztem wprowadzania jednej zmiennej znacznikowej to uczciwa transakcja.

W przypadku naprawdę skomplikowanych warunków, gdzie znaczników/ifów/switchów musielibyśmy mieć mnóstwo, lepszym rozwiązaniem mogłoby być użycie tabel decyzyjnych, choć i one czasami niezbyt się nadają (zależy tu od dziedziny testowanych danych).

Jak zwykle, dużo zależy od konkretnego przypadku i przesadzenie w którąkolwiek ze stron jest złe. Często trzeba szukać jakiegoś kompromisu. Ja np. wbrew pozorom nie lubię spamować zmiennymi i znacznikami i do wyjścia z pętli wolę użyć break lub return. W tym wypadku nie skorzystanie znacznika uważam przeważnie za korzystniejsze i polepszające czytelność kodu. Łamię w ten sposób klasyczną zasadę programowania czysto strukturalnego (o jednym punkcie wyjścia), ale uważam -- podobnie jak wielu autorów (Robert C. Martin, Steve McConnell...) to często za lepsze rozwiązanie. Natomiast DRY jest tak silną i uniwersalną zasadą, że powinno się ją naginać tylko w wyjątkowych przypadkach.

Ten konkretny przypadek łatwo przekształcić tak, jak napisałem. Bo mamy już zmienne a, b, wynik, a nawet używamy zmiennej c do wypisania działania. Refaktoryzacja jest więc stosunkowo nieskomplikowana. Jeśli dojdą nam jakieś kolejne dziwne warunki czy operacje matematyczne ulegną takiej zmianie, że wypisywanie wyniku nie było już takie spójne, to wtedy będzie można pomyśleć o innej strukturze kodu. Ja bym tego na pewno nie zostawił tak, jak jest teraz. Gdy wewnątrz procedury używasz kopiuj/wklej, w dodatku pięciokrotnie, to coś jest nie tak...

PS. Kiedyś byłem na rozmowie kwalifikacyjnej, czy raczej testach kwalifikacyjnych. Jednym z rozgrzewkowych pytań był klasyczny FizzBuzz ( http://www.codinghorror.com/blog/2007/02/why-cant-programmers-program.html ). Czasu było mało, więc napisałem na szybko (na kartce :) ) prostą wersję bez znacznika, wynikającą wprost z treści zadania. Sprawdzający spytał mnie się, jak mógłbym wyeliminować jeden warunek, czy też drobne powtórzenie kodu. Było kilka rozwiązań, ale podałem w końcu wersję ze znacznikiem i o to właśnie chodziło pytającemu. W sumie przypadek całkiem podobny do tego tutaj -- też chodziło o wypisywanie na ekran, ale instrukcja była dużo prostsza i nie była powtarzana 5x.

1 użytkowników online, w tym zalogowanych: 0, gości: 1