Złe działanie programu szukającego największej wartości w tablicy

Question

Piszę program który tworzy tablicę losowo stworzonych elementów i ma odszukać największą wartość w tej tablicy.

Założenie było takie, że funkcja searching_highest w pętli for po kolei porównuje ze sobą elementy [i] i [i+1] i jeśli pierwszy jest większy od drugiego, to nadpisuje go do zmiennej highest. 

Program bardzo dziwnie się zachowuje, poniżej kod: 

#include <cstdlib>
#include <ctime>
#include <iostream>

using namespace std;

void show_table(int table[], int table_size);
void searching_highest(int table_size, int* table);

int main ()
{
    srand(time(NULL));
    int table[50];
    for (int i=0; i<50; i++)
    {
        table[i] = rand() % 100;
    }
    show_table(table, 50);
    cout<<endl;
    cout<<"Highest table element: ";
    searching_highest(50, table);
}

void searching_highest(int table_size, int* table)
{
    int highest;
    for (int i=0; i<table_size; i++)
    {
        if (table[i]>table[i+1])
        {
        int buffer=table[i];
        highest=table[i];
        table[i]=buffer;
        }
        cout<<highest<<endl;
    }
}

void show_table(int table[], int table_size)
{
    cout<<"{";
    for (int i=0; i<table_size; i++)
    {
        if (i !=0)
        {
            cout<<", ";
        }
        cout<<table[i];
    }
    cout<<"}";
}

 

Comments

Nie wiem dlaczego wymyślać koło na nowo, popatrz na bibliotekę algorytmów algorithms, masz tam gotową funkcję max_element.

Dodatkowo poczytaj o random i chrono, które obowiązują już od c++11

Answer 1

Dużo masz błędów w tym kodzie. Funkcja searching_highest powinna wyglądać tak:

void searching_highest(int table_size, int* table)
{
    int highest = table[0];

    for (int i = 0; i < table_size - 1; ++i)
        if (highest < table[i + 1])
            highest = table[i + 1];

    cout << highest << endl;
}

Zamiast porównywać elementu o indeksie i oraz i+1 powinieneś porównywać każdy element z zmienną highest , wiadomo dlaczego. Nie rozumiem czemu użyłeś zmiennej buffer , której nazwa wskazuje, że następuje buforowanie skoro tutaj żadnego buforowania nie ma. Dziwne jest to, że u ciebie wyświetlenie zmiennej highest następuje przy każdej iteracji skoro powinna być tylko raz po znalezieniu największej wartości. Inną ważną rzeczą jaką trzeba powiedzieć jest to, że u ciebie pętla w ostatniej iteracji porównuje element o indeksie 49 z elementem o indeksie 50 skoro, w tej tablicy nie ma elementu 50 , więc warunkiem działania pętli powinno być "i < table_size - 1" a nie "i < table_size ".

Comments

ja bym od siebie dodał że można zwrócić int* , a highest ustawić jako wskaźnik do tablicy, tak jak to jest zrobione w STL chyba. Np if (* highest < table[i]) ) higest=&table[i];

---

Tak to jest dobry pomysł. W sumie w tym jego kodzie to można by wiele rzeczy zmienić... Ewentualnie zamiast wskaźnika było można by też zwrócić referencję do największej wartości.

---

Już rozumiem błędy jakie popełniłem, teraz wydają mi się tak oczywiste, że aż śmieszne :) Dziękuję za pomoc. 

 

Nie rozumiem tylko dlaczego w funkcji musiałem dać int* table, widziałem podobną funkcję w książce z której się uczę i tam było bez wskaźnika. O ile to w ogóle jest wskaźnik, bo do tego rozdziału jeszcze nie dotarłem. Tak samo u mnie w funkcji show_table jest w argumentach int table[] bez wskaźnika.

---

Dać w funkcji "int* table" w sensie jako parametr funkcji, o to chodziło? Jeśli tak to użycie wskaźnika (int*) jest wydajniejsze niż przekazanie przez kopie tablicy (czyli int[]) , ponieważ dzięki wskaźnikowi unikamy kopiowania tablicy oraz pracujemy na oryginalnych wartościach, a nie na kopiach.

Answer 2

i jeśli pierwszy jest większy od drugiego, to nadpisuje go do zmiennej highest. 

A to po co przypisujesz cokolwiek do tablicy tutaj?

table[i]=buffer;

W każdym razie...

for (int i=0; i<table_size; i++)
    if (table[i]>table[i+1])

Zastanów się, co się tutaj dzieje w ostatniej iteracji pętli.

cout<<highest<<endl;

Wywołujesz to w pętli?