• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Pomoc w czystym kodzie - zadanie z kursu najblizsza sredniej.

VPS Starter Arubacloud
0 głosów
334 wizyt
pytanie zadane 9 czerwca 2020 w C i C++ przez plolon20 Nowicjusz (220 p.)

Cześć, to mój pierwszy post, zaczynam dopiero programowanie i naukę języka C++, chciałbym usłyszeć pierwsze porady co mógłbym zmienić w kodzie i co poprawić, jestem pewien, że to zadanie można zrobić o wiele lepiej i bardziej "przejrzyście". Poniżej wstawiam kod i jeżeli ktoś znajdzie chwilę mógłby wskazać miejsce w kodzie i co poprawić z góry dzięki za pomoc smiley.

 

#include <iostream>

using namespace std;
float liczba[5];
float srednia=0;
float wartosc[5];
float bliska;
int main()
{
    cout << "Podaj 5 liczb oddzielonych spacja: " << endl;
    for (int i=0; i<5; i++) {
        cin>>liczba[i];
}
    for (int i=0; i<5; i++) {
        srednia+=liczba[i];
    }
    srednia/=5;
    cout<<"Srednia z tych liczb wynosi: "<<srednia<<endl;

    for (int i=0; i<5; i++) {
        wartosc[i]=srednia-liczba[i];
        if (wartosc[i]<0) wartosc[i]=-wartosc[i];
    }
    bliska = wartosc[0];
    for (int i=1; i<5; i++) {
        if (wartosc[i]<bliska) bliska=wartosc[i];
    }
    for (int i=0; i<5; i++) {
        if (liczba[i]==liczba[i-1]||liczba[i]==liczba[i-2]||liczba[i]==liczba[i-3]||liczba[i]==liczba[i-4]) continue;
        if (bliska==wartosc[i]) cout<<"Najblizsza sredniej jest liczba: "<<liczba[i]<<endl;

    }

    return 0;
}

Program zadziałał dobrze w każdym przypadku, jednak chciałbym go i tak udoskonalić.

5 odpowiedzi

+1 głos
odpowiedź 10 czerwca 2020 przez plolon20 Nowicjusz (220 p.)

Dzięki wszystkim za pomoc, jednak wasze kody, mimo, że są ładniejsze i czytelniejsze i na pewno mają o wiele więcej sensu, to mój kod jest lepszy pod tym względem, że działa prawidłowo. Gdy do waszego kody wpiszemy liczby "3.5 6.5 0 15 0" to wyjdzie wynik jedynie jednej liczby np 6.5, a najblizej sredniej znajduje sięzarówno 3.5 jak i 6.5 dlatego mój kod na końcu ma taką dziwną postać. Nie wiedziałem jak to rozwiązać inaczej, dzisiaj jeszcze pogłówkuje i podeśle coś.

Oczywiście zastosuję się od razu do waszych innych porad dzięki jeszcze raz laugh

nanautzinTOM_CPPmokrowski.

0 głosów
odpowiedź 9 czerwca 2020 przez nanautzin Obywatel (1,510 p.)
Na pewno możesz pomyśleć o zmniejszeniu ilości pętli
Musisz również pomyśleć jak zautomatyzować ostatnią pętle, w tym przypadku masz tylko 5 liczb, a co jak będziesz miał 100 czy 1000, będziesz klepał 1000 warunków?

Mógłbym ci podsunąć jak ja bym to rozwiązał, ale więcej się nauczysz jak sam pokombinujesz
0 głosów
odpowiedź 10 czerwca 2020 przez TOM_CPP Pasjonat (22,640 p.)
  1. Nie używaj zmiennych globalnych, czyli takich które zdefiniowane są poza ciałem funkcji main.
  2. W linii 29 dane odczytywane są z poza zakresu tablicy liczba.

Całość można napisać krócej, używając mniejszej liczby zmiennych i tylko dwóch pętli.

#include <iostream>

using namespace std;

int main()
{
    const int wymiar_tablicy = 5;

    float liczba[wymiar_tablicy];
    float srednia {0};
    int bliska {0};

    cout << "Podaj " << wymiar_tablicy << " liczb oddzielonych spacja: " << endl;
    for( int i {0} ; i<wymiar_tablicy ; ++i )
    {
        cin >> liczba[i];
        srednia += liczba[i];
    }

    srednia/=wymiar_tablicy;

    cout << "Srednia z tych liczb wynosi: " << srednia <<endl;

    for( int i=1 ; i<wymiar_tablicy ; ++i )
    {
        if( abs(srednia-liczba[i])<abs(srednia-liczba[bliska]) ) bliska=i;
    }

    cout << "Najblizsza sredniej jest liczba: " << liczba[bliska] << endl;

    return 0;
}

 

0 głosów
odpowiedź 10 czerwca 2020 przez mokrowski Mędrzec (156,260 p.)

Jeśli pisał byś to "produkcyjnie", to zapewne powinno wyglądać to tak:

#include <iostream>
#include <algorithm>
#include <numeric>
#include <cstddef>
#include <iterator>
 
using namespace std;
 
int main()
{
    constexpr static size_t WYMIAR_TABLICY = 5;
 
    float liczba[WYMIAR_TABLICY];
 
    cout << "Podaj " << WYMIAR_TABLICY << " liczb oddzielonych spacja:\n";

    copy_n(istream_iterator<float>(cin), WYMIAR_TABLICY, begin(liczba));
    auto srednia = accumulate(begin(liczba), end(liczba), 0.0) / WYMIAR_TABLICY;
    auto bliska = accumulate(begin(liczba) + 1, end(liczba), *begin(liczba), [&](const auto a, const auto b) {
		return abs(srednia - a) < abs(srednia - b) ? a: b;
    });
 
    cout << "Srednia z tych liczb wynosi: " << srednia << '\n'
    	 << "Najblizsza sredniej jest liczba: " << bliska << '\n';
}

A co do uwag w Twoim kodzie:

1. Nie stosuj "wartości magicznych". W Twoim kodzie to wartość 5. W przypadku zmiany wymagania (np. więcej danych), będzie konieczność poprawy w wielu miejscach. Stąd lepiej zdefiniować stałą i jej używać.

2. Dziel kod na funkcje jeśli nie jest to pojedyncze wywołanie np. algorytmu. Poprawia to czytelność i informuje jakie masz intencje. Np. funkcja oblicz_srednia(...) już jest lepsza niż klepanie pętli w pojedynczym spagetti kodzie. Oczywiście lepiej  używać angielskiego.

3. Nie stosuj zmiennych globalnych. To zła praktyka.

4. Nie nadużywaj std::endl. W większości wypadków, wystarcza tylko znak nowej linii '\n'.

5. W pętlach staraj się preferować pre inkrementację (++i) , ponad post inkrementację (i++).

6. Nazywaj poprawne zmienne. Np. srednia to na początku nie średnia a suma.

komentarz 10 czerwca 2020 przez nanautzin Obywatel (1,510 p.)
Mógłbyś przybliżyć punkt 5, dlaczego?
komentarz 10 czerwca 2020 przez mokrowski Mędrzec (156,260 p.)
1. Post inkrementacja, sugeruje konieczność dostępu do poprzedniej wartości zmiennej, co w typowej pętli for, nie ma miejsca.

2. Post inkrementacja sugeruje kompilatorowi by pozostawił starą wartość w rejestrze, co może prowadzić do nieefektywnego wykorzystania zasobów CPU.

3. Niektórzy twierdzą że "dobry kompilator danej platformy powinien to zoptymalizować", problem tylko z definicją "dobry", jakiej "danej platformy" i co oznacza "powinien".
komentarz 10 czerwca 2020 przez tkz Nałogowiec (42,020 p.)
Tak z ciekawości, jest jakiś benchmark post i pre inkrementacji w pętli for? Nie znam żadnego kompilatora dla którego miało by to znaczenie, czy dostanie ++i, czy i++. Co oczywiście nie implikuje, że go nie ma. Sprawdziłem typy POD w pętli dla GCC 10.x i wynik przy optymalizacji O1 był taki sam, wynik rozumiem jako kod wynikowy w assemblerze.
komentarz 10 czerwca 2020 przez mokrowski Mędrzec (156,260 p.)
A czy będzie miało znaczenie jeśli typem i będzie klasa z przeciążonymi operatorami pre i post? Jak sprawdzisz implementację przeciążenia, zorientujesz się że ma to znaczenie.

Rozważ... pytasz o rodzaj kompilatora, platformę i opierasz zdanie o "je nie znam", a na drugim końcu masz czytelność kodu i logikę :)

Osobiście spotkałem się z 3 kompilatorami które były tak proste że nie wykonywały tej optymalizacji. To były platformy embedded i DSP.
komentarz 10 czerwca 2020 przez tkz Nałogowiec (42,020 p.)
W mojej opinii takie rozważania mają sens właśnie tylko przy specjalnych platformach jakimi są systemy wbudowane. Przy x86, czy ARM raczej opierałbym się na gwarancjach samego języka, bo o ile wiem te dwie architektury nie zawierają specjalnej instrukcji inkrementacji, jeżeli jest inaczej, to proszę poprawić. Oczywiście dla typów nie POD, to zależy, przyznaje rację pisząc, że odnoszę się właśnie do typów podstawowych. Żeby też nie było, że walczę co jest lepsze, bo uważam, że to zależy. Po prostu warto wspomnień o tym, że obecnie kompilatory takie jak msvc i gcc na pewno to zoptymalizują dla trywialnych konstruktorów kopiujących i wyżej wspomnianych typów podstawowych.
komentarz 10 czerwca 2020 przez mokrowski Mędrzec (156,260 p.)

A rób jak chcesz w kodzie który nie jest produkcyjny (zresztą może nie tę ale inne reguły we własnych projektach traktuję uznaniowo) :)

https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement

https://www.embedded.com/pre-increment-or-post-increment-in-c-c/

Pytanie dotyczyło dobrych praktyk a nie "zróbmy jak uważamy". To ostatnie może być ustalone w danym projekcie/firmie/branży.

A co do pytania:

Przy x86, czy ARM raczej opierałbym się na gwarancjach samego języka, bo o ile wiem te dwie architektury nie zawierają specjalnej instrukcji inkrementacji, jeżeli jest inaczej, to proszę poprawić.

Oczywiście że zawierają. Prawdę mówiąc, trudno będzie Ci znaleźć jakiekolwiek ISA które nie ma inc/dec.

Po prostu warto wspomnień o tym, że obecnie kompilatory takie jak msvc i gcc na pewno to zoptymalizują dla trywialnych konstruktorów kopiujących i wyżej wspomnianych typów podstawowych.

A w pkt 3. komentarza wspomniałem :) Jeszcze mogę tylko dodać "na danym poziomie optymalizacji (jakim) lub bez".

komentarz 10 czerwca 2020 przez tkz Nałogowiec (42,020 p.)

Oczywiście że zawierają. Prawdę mówiąc, trudno będzie Ci znaleźć jakiekolwiek ISA które nie ma inc/dec.

To muszę doczytać o tym. Dzięki. 

komentarz 10 czerwca 2020 przez tkz Nałogowiec (42,020 p.)

Fakt, x86 posiada odpowiednią instrukcję 

The lodsb instruction does the equivalent of mov al,[esi] followed by inc esi.

0 głosów
odpowiedź 11 czerwca 2020 przez plolon20 Nowicjusz (220 p.)

Dobra poprawiłem kod wszystko działa, wydaje mi się, że wygląda dużo lepiej zostawiam do oceny i oczywiście mam jeszcze jedną prośbę. Czy mógłby ktoś pomóc stworzyć ostatni warunek w kodzie, tak aby pomijał te same liczby np gdy podam 3 3 3 3 3, program poda wszystkie te liczby jako bliskie średnie ja chciałbym, żeby podaj tylko 1 raz.

#include <iostream>
#include <cstdlib>

using namespace std;


int main()
{

    const int ilosc=5;
    float liczba[ilosc];
    float suma=0;
    float srednia;
    float bliska;

    cout << "Podaj 5 liczb oddzielonych spacja: \n";

    for (int i=0; i<ilosc; ++i)
    {
        cin>>liczba[i]; suma+=liczba[i];
    }

    srednia=suma/ilosc;

    cout<<"Srednia z tych liczb wynosi: "<<srednia<<endl;

    bliska=abs(srednia-liczba[0]);

    for (int i=1; i<ilosc; ++i)
    {
        if (abs(srednia-liczba[i])<bliska) bliska=abs(srednia-liczba[i]);
    }
    for (int i=0; i<ilosc; ++i)
    {
        if (abs(srednia-liczba[i])==bliska) cout<<"Najblizej sredniej znajduje sie liczba: "<<liczba[i]<<endl;;
    }


    return 0;
}

 

Podobne pytania

0 głosów
4 odpowiedzi 856 wizyt
pytanie zadane 20 marca 2016 w C i C++ przez Eliro Stary wyjadacz (12,160 p.)
0 głosów
2 odpowiedzi 870 wizyt
pytanie zadane 19 lipca 2018 w C i C++ przez Nowacx02 Obywatel (1,060 p.)
+2 głosów
2 odpowiedzi 1,039 wizyt

93,030 zapytań

141,992 odpowiedzi

321,294 komentarzy

62,376 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj

Wprowadzenie do ITsec, tom 2

Można już zamawiać tom 2 książki "Wprowadzenie do bezpieczeństwa IT" - będzie to około 650 stron wiedzy o ITsec (17 rozdziałów, 14 autorów, kolorowy druk).

Planowana premiera: 30.09.2024, zaś planowana wysyłka nastąpi w drugim tygodniu października 2024.

Warto preorderować, tym bardziej, iż mamy dla Was kod: pasja (użyjcie go w koszyku), dzięki któremu uzyskamy dodatkowe 15% zniżki! Dziękujemy zaprzyjaźnionej ekipie Sekuraka za kod dla naszej Społeczności!

...