paweljumper Temat założony przez niniejszego użytkownika |
Optymalizacja kodu prymitywnej bazy » 2017-08-22 22:29:34 Witam, napisałem taki prosty program który pozwala na modyfikowanie tablicy struktur w pętli, ale chciałbym się dowiedzieć czy ten kod można uczynić czytelniejszym i czy można go w jakiś sposób zoptymalizować. BTW z góry przepraszam za założenie tego samego tematu w dziale kurs C++ - pierwsza pomoc. Po czasie zauważyłem że ten temat nadaje się raczej do umieszczenia w tym dziale ale nie udało mi się go z tamtąd usunąć. Z góry dzięki za pomoc :) main.cpp #include <iostream> #include "golf.h"
extern const int cLen; extern const int ctabSize;
using namespace std;
int main() { char name[ cLen ]; golf players[ ctabSize ] { "", 0 }; int hc; cout << "Podawaj nazwiska oraz handicapy graczy..." << endl; while( true ) { menu( players, ctabSize ); } return 0; }
#include "golf.h" #include <iostream> #include <cstring>
using std::cout; using std::endl; using std::cin; using std::string;
void makestr( char ch, int signs ) { for( int i = 0; i < signs; ++i ) { cout << ch; } cout << endl; }
string makestr( string str, char ch, int signs ) { str = ""; for( int i = 0; i < signs; ++i ) { str[ i ] = ch; } return str; }
void setgolf( golf & g, const char * name, int hc ) { if( name == '\0' ) { cout << "Pusty parametr w konstruktorze (nazwisko)" << endl << "Podaj prawidlowy parametr: "; while( !( cin.get( g.fullname, cLen ) ) ) { cin.clear(); cin.ignore(); cout << "Podaj prawidlowy parametr: "; } } else { strncpy( g.fullname, name, 40 ); } if( hc <= 0 || !hc ) { cout << "Zly handicap w konstruktorze, podaj prawidlowy: "; while( !( cin >> hc ) || hc <= 0 || hc == '\0' ) { cin.clear(); cin.ignore(); cout << "Podaj prawidlowy handicap: "; } } g.handicap = hc; g.fullname[ 0 ] = toupper( g.fullname[ 0 ] ); }
void setgolf( golf & g ) { cout << "Podaj nazwisko: "; if( !( cin.get( g.fullname, 40 ) ) ) { while( !( cin.get( g.fullname, 40 ) ) ) { cin.clear(); cin.ignore(); } } cout << "Podaj handicap: "; if( g.handicap >= 0 || !( cin >> g.handicap ) ) { while( !( cin >> g.handicap ) || g.handicap <= 0 ) { cin.clear(); cin.ignore(); cout << "Handicap nie moze byc ani pusty ani ujemny: "; } } g.fullname[ 0 ] = toupper( g.fullname[ 0 ] ); }
void deletegolf( golf & g ) { strncpy( g.fullname, "", 40 ); g.handicap = 0; }
void handicap( golf & g, int hc ) { g.handicap = hc; if( g.handicap <= 0 ) { cout << "Zly handicap w konstruktorze, podaj prawidlowy: "; while( !( cin >> g.handicap ) || g.handicap <= 0 ) { cin.clear(); cin.ignore(); cout << "Podaj prawidlowy handicap: "; } } }
void showgolf( const golf & g ) { cout << "Gracz " << g.fullname << ", handicap: " << g.handicap << endl; }
void showall( const golf g[], int tSize ) { cout << "Oto nasi zawodnicy: " << endl; int i; int count = 0; for( i = 0; i < tSize; ++i ) { if( strcmp( g[ i ].fullname, "" ) ) { if( i == 0 ) makestr( '-', 20 ); cout << i + 1 << ". " << g[ i ].fullname << ", " << g[ i ].handicap << endl; ++count; } } if( count == 0 ) cout << "Brak." << endl; else makestr( '-', 20 ); }
void menu( golf g[], int tSize ) { int manage; const golf * ptable = g; cout << "Wybierz 1 aby dodac gracza, 2 aby usunac i 3 aby wyswietlic graczy: "; while( !( cin >> manage ) ) { cin.clear(); cin.ignore(); cout << "Podaj 1, 2 lub 3!"; } if( manage == 1 ) { int i = 0; while(( strcmp( g[ i ].fullname, "" ) ) ) ++i; cout << "Wolne miejsce: " << i + 1 << endl; setgolf( g[ i ] ); showgolf( g[ i ] ); } else if( manage == 2 ) { showall( ptable, tSize ); int id; cout << "Podaj id gracza ktorego chcesz usunac: "; while( cin >> id && id < 0 ); id -= 1; deletegolf( g[ id ] ); } else if( manage == 3 ) { showall( ptable, tSize ); } else { cout << "Bledne dane :(" << endl; } }
|
|
darko202 |
» 2017-08-23 10:42:17 1. Nie zamieściłeś używanej struktury golf :( 2. nie jesteś konsekwentny w tym co chcesz osiągnąć: extern const int cLen; ... char name[ cLen ]; ... strncpy( g.fullname, name, 40 ); ...
czyli 40 to powód że dla (cLen == 50) program nie działa poprawnie no i stąd niekonsekwencja w celu jaki realizowałeś deklarując tak cLen 3. podobnie w cin.get( g.fullname, 40 ) 4. zastanawiający jest if( name == '\0' ) gdy const char * name 5. zastanowił mnie też tekst "Pusty parametr w konstruktorze (nazwisko)" bo w strukturze trudno mówić o konstruktorze teoretycznie można, ale mnie razi :) 6. podobnie cout << "Podaj handicap: "; if( g.handicap >= 0 || !( cin >> g.handicap ) warunek przed pobraniem wartości :( 7. spróbuj zdebugować kod, bo w kilku miejscach masz dziwne konstrukcje zastanawiające jest, czy ten kod działa ? 8. >> czy można go w jakiś sposób zoptymalizować ? używasz strncpy czyli -> C string to be copied. http://www.cplusplus.com/reference/cstring/strncpy/a mówisz o konstruktorach, czyli myślisz o obiektowości czyli -> c++ i wtedy można używać stringów trudno coś więcej powiedzieć nie znając tematu jaki realizujesz ograniczeń jakie masz narzucone |
|
paweljumper Temat założony przez niniejszego użytkownika |
» 2017-08-23 17:09:02 Oto plik ze strukturą :) #ifndef GOLF_H_INCLUDED #define GOLF_H_INCLUDED
const int cLen = 40; const int ctabSize = 4; struct golf { char fullname[ cLen ]; int handicap; };
void setgolf( golf & g, const char * name, int hc );
void setgolf( golf & g );
void deletegolf( golf & g );
void handicap( golf & g, int hc );
void showgolf( const golf & g );
void showall( const golf g[] );
void menu( golf g[], int tsize );
#endif
To nie jest większy projekt tylko przerobione zadanie w którym w sumie były tylko 2 funkcje setgolf, handicap i deletegolf. Dorobiłem tylko do niego menu. Co do pkt 6 to jesli odwroce ifa lub wrzuce cin przed petle i na koniec bloku petli to program się zwiesza :(. Wszystko działa jednak chciałem się dowiedzieć czy jest jakiś sposób na poprawę czytelności, przeczytałem coś o dzieleniu się kodem i tak jakoś wyszło :).Po prostu czy nie dało się czegoś zrobić prościej. |
|
DejaVu |
» 2017-08-23 18:18:08 Nie wykonuj optymalizacji, jeżeli nie widzisz ku temu racjonalnego uzasadnienia. Jeżeli nie jesteś w stanie zrobić benchmarka, który pokazuje występowanie problemu wydajnościowego, to po prostu nie wykonuj żadnych optymalizacji. |
|
mateczek |
» 2017-08-23 19:53:05 optymalizacji wydajnościowej może i nie trzeba. Ale poukładać kod w klasy, poprawić czytelność. Lub zrobić tak, by w przyszłości kod nadawał się do ponownego wykorzystania nigdy nie zaszkodzi. |
|
darko202 |
» 2017-08-24 11:44:05 1. >>Co do pkt 6 to jesli odwroce ifa lub wrzuce cin przed petle i na koniec bloku petli to program się zwiesza :(. trudno się do tego ustosunkować, bo musielibyśmy wszyscy zobaczyć ten kod a tak można zgadywać ponieważ piszesz że metodę tą już miałeś to rozumiem, że ktoś popełnił błąd który Ty powielasz niestety w tej chwili nie realizuje spodziewanej funkcjonalności :) dobra praktyka jest na pisanie kodu który działa dla wszystkich możliwych wartości, a nie tych wymyślonych przez programistę przypuszczam, że obserwowane zawieszenie programu to wejście programu w pętlę nieskończoną taka jak np. do { } while( warunek_konczacy );
2. rozumiem, że chcesz się rozwinąć, dlatego podsunę Ci tematy do przemyśleń * metody makestr nie są związane ze strukturą golf dlatego powinny znaleźć się w innym pliku * metoda deletegolf to funkcjonalnie clear metoda delete oznacza zwolnienie zarezerwowanej pamięci * jeśli mówimy (myślimy) o programowaniu obiektowym - nawet na strukturze to domyślnie w nazwach nie używamy nazwy obiektu tzn. np. metody nowy(), dodaj(), usun(),... a nie nowyGolf(), dodajGolf(), usunGolf(),... * metoda handicap jest źle skonstruowana (zła praktyka) 1. zmieniasz wartość zmiennej na podaną 2. sprawdzasz czy jest prawidłowa 3. jeśli zła to próbujesz uzyskać poprawną i zmienić to co zrobiłeś w punkcie 1 punkt 1 powinien być wykonany dopiero po spełnieniu warunku z punktu 2 * unikaj mieszania np. czytaniu ze strumienia z warunkami poprawności danych, sprawdzenia przeczytania czegoś ze strumienia jak w : a) while( !( cin >> g.handicap ) || g.handicap <= 0 ) b) while( !( cin >> hc ) || hc <= 0 || hc == '\0' ) gdy || to lub spójrz a potem zamknij oczy i szybko powiedz co sprawdziłeś w tym warunku * zapoznaj się z techniką debagowania kodu niezbędne, by wiedzieć co dzieje się w konkretnej linii programu (np. zobaczyć stan zmiennych, ustalić wartość warunku wyjścia z pętli) http://kobietydokodu.pl/26-debugowanie-aplikacji/https://www.visualstudio.com/pl/vs/getting-started/desktop-debug/ |
|
f651144 |
» 2017-08-29 17:07:20 Cóż, zagadam się z poprzednikami - przedwczesna optymalizacja nie ma sensu. Jak rozumiem chodzi Ci też jednak o uporządkowanie kodu i napisanie w sposób obiektowy lub po prostu inny. Sposobów na to jest bardzo wiele. Nim jednak przejdziemy do tego zwrócę uwagę na jedną istotą rzecz - pisanie w ten sposób (i tworzenie takich konstrukcji): extern const int cLen; extern const int ctabSize;
const int cLen = 40; const int ctabSize = 4;
jest bardzo, bardzo brzydkie, nie przenoście i nieładnie pachnie ( dosłownie). Nie wspominając już, że powinno być odwrotnie, tj. to co jest w pliku źródłowym powinno być w nagłówku, a to co w nagłówku powinno być w pliku źródłowym. Ale przejdźmy do rzeczy - sposobów na zmianę Twojego kudu (tj. jego refaktoryzację) jest bardzo wiele. Je osobiście uwielbiam metaprogramowanie. Jeśli nie spotkałeś się z tym terminem, to ogólnie rzecz biorąc jest to związane z szablonami i generowaniem kodu na etapie kompilacji. Sprawa jednak (zwłaszcza w obliczu najnowszych standardów) jest dość skomplikowana. Ponad to kod szablonowy (w przypadku dużych aplikacji) potrafi spowolnić proces kompilacji. Jest jednak bardzo wiele korzyści: Poniżej zamieszczam rezultat mojej zabawy z Twoim programem. Podkreślam, że ten kod jest zrobiony na szybko, bez specjalnego ładu. Mam jednak nadzieję, że będzie dla Ciebie ciekawą lekturą. #ifndef GOLF_HPP_INCLUDED #define GOLF_HPP_INCLUDED
#include <ostream> #include <utility>
template < typename T, T Upper, T Lower > struct limits { using type = T; static constexpr inline T upper = Upper; static constexpr inline T lower = Lower; };
template < typename Name, typename NameLimits, typename Handicap, typename HandicapLimits > class golf { public: using name_type = Name; using handicap_type = Handicap; using name_limits = NameLimits; using handicap_limits = HandicapLimits; golf() = default; golf( const golf < Name, NameLimits, Handicap, HandicapLimits >& ) = default; golf( golf < Name, NameLimits, Handicap, HandicapLimits >&& ) = default; golf < Name, NameLimits, Handicap, HandicapLimits >& operator =( const golf < Name, NameLimits, Handicap, HandicapLimits >& ) = default; golf < Name, NameLimits, Handicap, HandicapLimits >& operator =( golf < Name, NameLimits, Handicap, HandicapLimits >&& ) = default; template < typename Other > bool fullname( Other && other ) { if( other.size() <= name_limits::upper && other.size() >= name_limits::lower ) { this->fullname_ = std::forward < Other >( other ); this->empty_ = false; return true; } return false; } template < typename Other > bool handicap( Other && other ) { if( other <= handicap_limits::upper && other >= handicap_limits::lower ) { this->handicap_ = std::forward < Other >( other ); this->empty_ = false; return true; } return false; } const name_type & fullname() const { return this->fullname_; }; const handicap_type & handicap() const { return this->handicap_; }; void clear() { this->empty_ = true; }; bool empty() const { return this->empty_; }; private: name_type fullname_; handicap_type handicap_; bool empty_ = true; };
template < typename Name, typename NameLimits, typename Handicap, typename HandicapLimits > std::ostream & operator <<( std::ostream & os, const golf < Name, NameLimits, Handicap, HandicapLimits >& g ) { os << "Gracz "; os.write( g.fullname().data(), g.fullname().size() ); os << ", handicap: " << g.handicap(); return os; }
#endif
#ifndef UTILS_HPP_INCLUDED #define UTILS_HPP_INCLUDED
#include <algorithm> #include <array> #include <iostream> #include <iterator> #include <limits> #include <sstream> #include <string>
template < typename String > String get_line() { String res; std::getline( std::cin, res ); std::cin.clear(); return res; }
template < typename To, typename String > bool str_cast( To & to, const String & str ) { if( str.size() == 0 ) return false; std::istringstream iss( str.c_str() ); iss >> to; if( iss && iss.eof() ) return true; return false; }
template < typename Golf > void set_handicap( Golf & g ) { using handicap_type = typename Golf::handicap_type; auto load =[ & g ]()->bool { handicap_type hc; return !str_cast < handicap_type >( hc, get_line < std::string >() ) || !g.handicap( hc ); }; std::cout << "Podaj handicap: "; while( load() ) std::cout << "Błędny handicap, podaj prawidlowy: "; }
template < typename Golf > void set_fullname( Golf & g ) { using fullname_type = typename Golf::name_type; auto load =[ & g ]()->bool { auto str = get_line < std::string >(); fullname_type fn( str.data(), str.size() ); return !g.fullname( fn ); }; std::cout << "Podaj nazwisko: "; while( load() ) std::cout << "Błędne nazwisko, podaj prawidlowe: "; }
template < typename GolfIterator > void display_all( GolfIterator begin, GolfIterator end ) { std::cout << "Oto nasi zawodnicy: " << std::endl; bool first = true; auto display =[ & first ]( size_t pos, const auto & g ) { if( first ) { std::fill_n( std::ostream_iterator < char >( std::cout ), 20, '-' ); std::cout << std::endl; } std::cout << pos + 1 << " " << g << std::endl; first = false; }; size_t count = 0; std::for_each( begin, end,[ & count, & display ]( const auto & g ) { if( !g.empty() ) display( count, g ); count += 1; } ); if( first ) std::cout << "Brak." << std::endl; else { std::fill_n( std::ostream_iterator < char >( std::cout ), 20, '-' ); std::cout << std::endl; } }
inline bool get_idx( size_t & idx, size_t limit ) { if( !str_cast < size_t >( idx, get_line < std::string >() ) ) return false; idx -= 1; if( idx >= limit ) return false; return true; }
template < typename GolfArray > void menu( GolfArray & array ) { std::array < void( * )( GolfArray & ), 3 > action; action.fill( nullptr ); action[ 0 ] =[]( GolfArray & a ) { bool stop = false; size_t i = std::count_if( a.begin(), a.end(),[ & stop ]( const auto & g )->bool { return !( stop =( stop ? true: g.empty() ) ); } ); if( i >= a.size() ) { std::cout << "Brak wolnego miejsca." << std::endl; return; } std::cout << "Wolne miejsce: " << i + 1 << std::endl; set_fullname( a[ i ] ); set_handicap( a[ i ] ); std::cout << a[ i ] << std::endl; }; action[ 1 ] =[]( GolfArray & a ) { display_all( a.begin(), a.end() ); size_t id; std::cout << "Podaj id gracza ktorego chcesz usunac: "; while( !get_idx( id, a.size() ) ) std::cout << "Podaj id gracza ktorego chcesz usunac: "; a[ id ].clear(); }; action[ 2 ] =[]( GolfArray & a ) { display_all( a.begin(), a.end() ); }; size_t idx = 0; std::cout << "Wybierz 1 aby dodac gracza, 2 aby usunac i 3 aby wyswietlic graczy: "; while( !get_idx( idx, action.size() ) ) std::cout << "Podaj 1, 2 lub 3: "; action[ idx ]( array ); }
#endif
#include <array> #include <iostream> #include <string>
#include "golf.hpp" #include "utils.hpp"
int main() { std::array < golf < std::string, limits < size_t, 40, 0 >, int, limits < int, 100, 0 >>, 4 > golf_array; std::cout << "Podawaj nazwiska oraz handicapy graczy..." << std::endl; while( true ) menu( golf_array ); return 0; }
Cały kody skompilowałem u siebie używając kompilatora z pakietu GCC w wersji 7.2.0 (kompilowałem z flagą -std=c++1z, więc nie jestem pewien jak bardzo kody jest kompatybilny wstecz). Z ciekawszych fragmentów: Jeżeli Twój kompilator jest za stary możesz użyć https://wandbox.org/ - trzeba tylko wrzucić wszystkie 3 pliki razem i zakomentować linie z includami w pliku main.cpp. Miłej zabawy! |
|
« 1 » |