Jak można polepszyć mój kod ?

Question

#include <iostream>
using namespace std;
int ilosc;
float liczba;
float suma;
float srednia;




int main()
{
    cout<<"Witaj ten program obliczy Ci srednia."<<endl;
    cout<<"Teraz musisz podac ile masz liczb : ";
    cin>>ilosc;
    cout<<"Teraz podaj Jakie masz liczby ."<<endl;
    for (int i=1; i<=ilosc; i++)
    {
        cout<<"Podaj "<<i<<" liczbe :";
        cin>>liczba;
        cout<<""<<endl;
        suma+=liczba ;
    }
    srednia=suma/ilosc ;
    cout<<"Twoja srednia to : "<<srednia ;


    return 0;
}

Jestem otwarty na wszelką krytykę i dobre rady.

Comments

czysto estetycznie

---

cout<<endl;

---

dzięki

---

Jeśli będziesz sumował całkowite użyj rzutowania typów

---

Chciałbym zwrócić uwagę, że std::endl nie tylko przełamuje linię, ale również wywołuje std::ostream::flush(). Czasem to jest okej, ale w większości niepotrzebnie wywołujemy flush().

Jeśli chce się tylko nowy wiersz, należy w strumieniu umieścić znak '\n'.

To taka rada, jakby ktoś kiedyś planował pisać coś na poważnie.

Answer 1

1. Nie używaj zmiennych globalnych;
2. Nie używaj using namespace std;
3. Dorzuć trochę enterów i spacji dla czytelności;
4. Taka pętla będzie lepsza: for (auto i = 0u; i < ilosc; ++i). Warto dodać, że ++i jest z reguły lepsze od i++.

Comments

Dlaczego nie używać using namespace?

---

Z większością się zgodzę, jednak poproszę wymierny powód dla punktu czwartego (preinkrementacja typu int/uint to nie jest powód ;).

---

Yep, jednak w tym wypadku nie tyle chodzi o prędkość co o świadomość co dzieje się w kodzie. ++zmienna – podbija wartość, najlogiczniejsze podejście w większości wypadków, zmienna++ – kopiuje zawartość, podbija to co jest w zmiennej i wtedy zwraca. Jeśli ktoś nie oczekuje specjalnie drugiego zachowania to powinien użyć tego pierwszego, nie sądzisz? No i jak wspomniałeś, typy podstawowe nie robią różnicy, ale jeśli postanowi zmienić to na iterator to nawet nie ma gwarancji czy istnieje operator postinkrementacji.

---

1. Auto zapewnia dużą elastyczność;
2. 0u uczytelnia kod wyjaśniając dodatkowo jakich wartości na pewno nie będzie miał licznik;
3. Iterowanie po zakresie [0; n) to w praktyce standard i swoista konwencja, taki kod często dużo lepiej się czyta i IMHO warto się tego trzymać.

Ps. Teraz zobaczyłem, że mam tam w kodzie literówkę, pętla miała oczywiście jechać w przedziale [0; n), nie [1, n), zaraz to poprawię.

---

Auto zapewnia dużą elastyczność;

Owszem, ale nie tutaj. Akurat ta pętla z jawną specyfikacją typu jest czytelniejsza, nie pozostawia pola na jakąkolwiek wątpliwość i jest krótsza. W czym tutaj auto jest bardziej elastyczne od int? Gdyby chcieć zmienić tę pętlę na zakresową, i tak całość idzie do kosza.

[EDIT] Oglądałem sobie ostatnio nagranie z CppCon2014, w którym Herb Sutter mówił m.in. o przeniesieniu typowania na prawą stronę. Dla zapewnienia jednolitości kodu, mogę się zgodzić z tym, że auto i = 0u; może być lepsze również tutaj, jednak nie ma to związku z elastycznością. ;) [/EDIT]

0u uczytelnia kod wyjaśniając dodatkowo jakich wartości na pewno nie będzie miał licznik;

[opinia]Wystarczy spojrzeć na wyrażenia sterujące, które są wystarczająco jasne.[/opinia]

Iterowanie po zakresie [0; n) to w praktyce standard i swoista konwencja, taki kod często dużo lepiej się czyta i IMHO warto się tego trzymać.

Pełna zgoda.