Optymalizacja kod (C++)

pytanie zadane 13 maja 2018 w C i C++ przez Sic Dyskutant (8,510 p.)

Witam kiedy napisałem program, który w pełni działa, jednak chciałbym go zoptymalizować za pomocą funkcji wbudowanych i bibliotek c++. Pytanie czy jest taka możliwość. Proszę o podpowiedź, bo dopiero się uczę poznawania bibliotek.

#include <iostream>

const int MAX = 5;

//prototypy funkcji
double *fill(double *ar, double * limit);//parametrami sa 2 wskazniki nie bedace tablicami
void show(double *ar, double * n);
void reva(double *r, double *ar, double n);

int main()
{
        double pro[MAX]; //utworzenie tablicy
        double *p = &pro[0]; // przypisanie wskaznikowi 'p' pierwszego elementu tablicy
        double *z = &pro[MAX - 1]; // przypisanie wskaznikowi 'z' ostatniego elementu tablicy
        double *wsk = fill(p, z); // wywolanie wskaznika na funkcje "fill"
        
        show(p, wsk);
        if(sizeof (pro) > 0) // ilosc bajtow tablicy musi byc wieksza od 0 '?' (ciekawe rozwiazanie)
        {
                std::cout << "Podaj czynnik zmiany wartosci: ";
                double f;
                while(!(std::cin >> f))
                {
                        std::cin.clear();
                        while (std::cin.get() != '\n')
                                continue;
                        std::cout << "Niepoprawna wartosc; podaj liczbe: ";     
                }

                reva(p, wsk, f);
                show(p, wsk);
        }
        std::cout << "GOTOWE.\n";
        
        std::cin.get();
        return 0;
}

double *fill(double *ar, double * limit)
{
        double temp;
        double *i; // dla poprawnosci konwersji typow 'i' musi byc typu double
        int licz = 1;
        std::cin >> temp;
        for(i=ar; i <= limit; i++)
        {
                std::cout << "Podaj wartosc nr " << licz << ": ";
                licz++;
                if(!std::cin)
                {
                        std::cin.clear();
                        while(std::cin.get() != '\n')
                                continue;
                        std::cout << "Bledne dane, wprowadzanie danych przerwane.\n";
                        break;
                }
                else if (temp < 0)
                        break;
                *i = temp; //przypisanie wskaznikowi zmienna "temp"
        }
        return i; // zwracanie wskaznika
}

void show(double *ar, double * n)
{
        for(double *i=ar; i < n; i++) // utworzenie wskaznika typu double (warto zapamietac taka mozliwosc tworzenia petli)
                std::cout << *i << std::endl;
        }
void reva(double * r, double * ar, double n)
{
        for(double *i=r; i < ar; i++) // jak w petli powyzej
                *i=n;           
}

3 odpowiedzi

odpowiedź 13 maja 2018 przez RafalS VIP (122,820 p.)

Przed optymalizacją zacząłbym od refaktoryzacji tzn - zmian programu, które nie zmienią samego działania. Strasznie się to czyta. Patrze na to i nie mam zielonego pojęcia co te funkcje robią. Zacznij od deskryptywnych nazw zmiennych i funkcji. Nie ma nic gorszego niż nazwy w stylu r, p, z. Weźmy pod lupę tę funkcję:

void reva(double * r, double * ar, double n)

wtf???
Nazewnictwo powinno być takie, że patrzysz na nazwe funkcji, zwracany typ, parametry i ich typy i mniej więcej masz się domyślać co ta funkcja będzie robić, bez zaglądania i analizowania kodu w środku. Tutaj mamy tego 100% przeciwieństwo. Nazwa reva, nie wiem na jakiej podstawie ją wymyśliłeś, ale mi ona nie mówi nic. Brzmi to jakby ta funkcja miała imie w stylu andrzej. Pierwszy parametr double* r. No spoko, wszystko jasne, serio. Drugi już lepiej bo dwie litery, i to ar, co sugeruje na array, jakąś tablice, ale co ta tablica ma do reszty parametrów to zielonego pojęcia. Ostatni - nazwa sugeruje na jakąś długość, ale typ przeczy tym domysłom; ponownie zielonego pojęcia po co ta zmienna.

Funkcje reva zrefactorowałbym do takiej deklaracji:

void setValueOfElements(double * startPointer, double * endPointer, double value);

Nazwa iteratora w pętli jest dopuszczalna jako i pod warunkiem, że pętla jest oczywista, czyli i jest intem i jest wykorzystywany do indeksowania tablicy. Ewentualnie nazwa it, gdy korzystamy z iteratorów.
Każde inne odstąpienie od normy musi mieć deskryptywną nazwę, bo kod będzie nieczytelny.

Te uwagi mogą Ci się wydawać dziwne, ale uwierz mi, jeśli kiedykolwiek ktoś będzie czytał Twój kod (nawet Ty sam za miesiąc, gdy zapomnisz o co chodziło) to podziękuje Ci, że jego słownictwo nie zejdzie do rynsztoka tak jak moje w tym momencie.

odpowiedź 14 maja 2018 przez mokrowski Mędrzec (155,460 p.)
edycja 14 maja 2018 przez mokrowski

Należy Ci się reprymenda za nazewnictwo. Wiem, kolega już to zrobił ale ... to co Tu odstawiłeś, jest "nieludzkie" :/

Propozycja nieco lepszego udokumentowania kodu z minimalnymi poprawkami błędów i ew. nieścisłości:

#include <iostream>
#include <limits>
#include <cstddef>

static const std::size_t MAX_ELEMENTS = 5;

double * set_table(double *first_ptr, double * last_ptr);
void show_table(double *first_ptr, double * last_ptr);
void fill_table(double *first_ptr, double * last_ptr, double value);

int main()
{
    double table[MAX_ELEMENTS];
    double * first_ptr = table;
    double * last_ptr = table + MAX_ELEMENTS;
    double * range_ptr = set_table(first_ptr, last_ptr);

    show_table(first_ptr, range_ptr);
    if(range_ptr - first_ptr)
    {
        std::cout << "Podaj czynnik zmiany wartosci: ";
        double value;
        while(!(std::cin >> value))
        {
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            std::cout << "Niepoprawna wartosc; podaj liczbe: ";
        }

        fill_table(first_ptr, range_ptr, value);
        show_table(first_ptr, range_ptr);
    }
    std::cout << "GOTOWE.\n";
}

double * set_table(double * first_ptr, double * last_ptr)
{
    double temp;
    int counter = 1;
    for(; first_ptr < last_ptr; ++first_ptr)
    {
        std::cout << "Podaj wartosc nr " << counter++ << ": ";
        if(!(std::cin >> temp) || (temp < 0)) {
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            std::cout << "Bledne dane, wprowadzanie danych przerwane.\n";
            break;
        }
        *first_ptr = temp;
    }
    return first_ptr;
}

void show_table(double * first_ptr, double * last_ptr)
{
    for(;first_ptr < last_ptr; ++first_ptr)
    {
        std::cout << *first_ptr << '\n';
    }
}
void fill_table(double * first_ptr, double * last_ptr, double value)
{
    for(;first_ptr < last_ptr; ++first_ptr)
    {
        *first_ptr = value;
    }
}

Dopiero teraz można zastanawiać się co z tym kodem zrobić. Co to za "Czynnik zmiany wartości"? Wartość na jaką zmienić (jak było w kodzie)?

I docelowo:

#include <iostream>
#include <limits>
#include <cstddef>
#include <string>
#include <algorithm>
#include <iterator>

static const std::size_t MAX_ELEMENTS = 5;

double * set_table(double *first_ptr, double * last_ptr);
bool read_double(double * value, const std::string& prompt, const std::string& error_msg);

int main()
{
    double table[MAX_ELEMENTS];
    double * first_ptr = table;
    double * last_ptr = table + MAX_ELEMENTS;
    double * range_ptr = set_table(first_ptr, last_ptr);

    std::copy(first_ptr, range_ptr, std::ostream_iterator<double>(std::cout, "\n"));
    if(range_ptr - last_ptr)
    {
        double value;
        while(!read_double(&value,
                    "Podaj czynnik zmiany wartości: ",
                    "Niepoprawna wartość, podaj liczbę\n"));

        std::fill(first_ptr, range_ptr, value);
        std::copy(first_ptr, range_ptr, std::ostream_iterator<double>(std::cout, "\n"));
    }
    std::cout << "GOTOWE.\n";
}

double * set_table(double * first_ptr, double * last_ptr)
{
    double temp;
    for(std::size_t counter = 1; first_ptr < last_ptr; ++counter, ++first_ptr)
    {
        if(!read_double(&temp,
                    "Podaj wartosc nr " + std::to_string(counter) + ": ",
                    "Bledne dane, wprowadzanie danych przerwane.\n")
                || (temp < 0))
        {
            break;
        }
        *first_ptr = temp;
    }
    return first_ptr;
}

bool read_double(double * value, const std::string& prompt, const std::string& error_msg)
{
    std::cout << prompt;
    std::cin >> *value;
    if(!std::cin) {
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << error_msg;
        return false;
    }
    return true;
}

komentarz 14 maja 2018 przez Sic Dyskutant (8,510 p.)
edycja 14 maja 2018 przez Sic

Dobrze zacząłem od poprawek nazw.

Jednak co oznacza to:

std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

A teraz kilka pytań co do optymalizacji:

while(!read_double(&value, "Podaj czynnik zmiany wartości: ",  "Niepoprawna wartość, podaj liczbę\n"));
 
std::fill(first_ptr, range_ptr, value);
std::copy(first_ptr, range_ptr, std::ostream_iterator<double>(std::cout, "\n"));
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

Dokumentację na temat tych funkcji przeczytałem, jednak co oznaczają te powyżej.

komentarz 14 maja 2018 przez Sic Dyskutant (8,510 p.)

Sprawa nr 2 (kompilije to w g++)

Pojawia się taki błąd:

so_7.cpp:(.text+0x115): undefined reference to `read_double(double*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
/tmp/cc4FEpd6.o: In function `set_table(double*, double*)':
so_7.cpp:(.text+0x35a): undefined reference to `read_double(double*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
collect2: error: ld returned 1 exit status

#include <iostream>
#include <limits>
#include <cstddef>
#include <string>
#include <algorithm>
#include <iterator>
const int MAX_ELEMENTS = 5;

double * set_table(double *first_ptr, double * last_ptr);
bool read_double (double * value, const std::string & prompt, const std::string & error_msg);

int main()
{
        double table[MAX_ELEMENTS]; 
        double *first_ptr = table; 
        double *last_ptr = table + MAX_ELEMENTS; 
        double *range_ptr = set_table(first_ptr, last_ptr);        
        
        std::copy(first_ptr, range_ptr, std::ostream_iterator<double>(std::cout, "\n")); // ?
        if(range_ptr - first_ptr) {
                double value;
                while(!read_double (&value,"Podaj czynnik zmiany wartości: ", "Nieprawidłowa wartość\n"));     
        
                std::fill(first_ptr, range_ptr, value);
                std::copy(first_ptr, range_ptr, std::ostream_iterator<double>(std::cout, "\n"));
        }  
        std::cout << "GOTOWE.\n";
}

double * set_table(double *first_ptr, double *last_ptr)
{
        double temp;
        for(std::size_t counter=1; first_ptr < last_ptr; ++counter, ++first_ptr) {
                if(!read_double (&temp, "Podaj wartość nr " + std::to_string(counter) + ": ", "Błędne dane, przerwanie\n") || (temp<0)) { break; }
        *first_ptr = temp;
        }
        return first_ptr; // zwracanie wskaznika
}

bool read_double (double * value, const std::string & prompt, std::string & error_msg)
{
        std::cout << prompt; std::cin >> *value;
        if(!std::cin) { std::cin.clear(); //czyszczenie
                std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
                std::cout << error_msg;
                return false;
        }       
        return true;
}

komentarz 15 maja 2018 przez mokrowski Mędrzec (155,460 p.)

Linia 10 zapowiada funkcję:

bool read_double (double * value, const std::string & prompt, const std::string & error_msg);

czyli: "przyjmująca wskaźnik na double i dwie stałe referencje na std::string".

Natomiast linia 60 definiuje:

bool read_double (double * value, const std::string & prompt, std::string & error_msg)

czyli: "przyjmująca wskaźnik na double, stałą referencję na std::string i nie-stałą referencję na std::string". Popraw 60 linię dodając const do 3 argumentu.

1. std::cin.ignore(... ) odrzuca maksymalnie X znaków ze strumienia aż do separatora i włącznie z nim. X jest w tym przypadku maksymalną długością strumienia znaków.

2. Linia:

while(!read_double(...

... dokonuje wczytania wartości double do 1 argumentu read_double(...) a sama funkcja zwraca true/false jeśli operacja się udała. W połączeniu z while, będzie pętlił aż wczyta poprawnie.

std::fill(....) wypełnia wartością z 3 argumentu zakres [start, stop) gdzie jak widać stop jest już poza zakresem wypełnianym. Standardowo std::fill stosuje się z iteratorami. Wskaźnik jest naturalnym iteratorem.

std::copy(początek1, koniec1, początek2), powoduje kopiowanie zakresu [początek1, koniec1), do początek2. Tak się jednak składa że początek2 jest iteratorem wyjścia na konsolę. Tak więc dane z zakresu [początek1, koniec1), trafią na konsolę.