michal11 |
» 2016-06-30 11:07:35 Moim zdaniem to: for( size_t i = 0; i < finalny.length(); i++ ) {
if( finalny[ i ] == '<' ) {
finalny[ i ] = '[';
}
else if( finalny[ i ] == '>' ) {
finalny[ i ] = ']';
}
}
nawet jeżeli jest krótsze to jest mniej czytelne od tego: for( size_t i = 0; i < finalny.length(); i++ )
{
if( finalny[ i ] == '<' )
{
finalny[ i ] = '[';
}
else if( finalny[ i ] == '>' )
{
finalny[ i ] = ']';
}
}
ew. przy tak krótkich ifach można jeszcze pomyśleć nad czymś takim: for( size_t i = 0; i < finalny.length(); i++ )
{
if( finalny[ i ] == '<' ) { finalny[ i ] = '['; }
else if( finalny[ i ] == '>' ) { finalny[ i ] = ']'; }
}
|
|
karambaHZP |
» 2016-06-30 14:19:49 Czepiając się szczegółów, kod nie ma zachowanej zasady SRP.
Funkcja konwertująca ma więcej niż jedną odpowiedzialność.
Funkcja konwertująca jest narzucona przez zadanie, ale można
dopisać kolejne (i wywołać je w funkcji konwertującej):
- zajmującą się podmianą znaków (argumentami są napis, znak szukany, nowy znak),
- zajmującą sie usuwaniem znaków (argumentami są napis, usuwany znak).
Dzięki temu zabiegowi mamy uniwersalne funkcje, które działają dla
różnych znaków. |
|
Elaine |
» 2016-07-01 05:42:13 Ale wtedy z kolei kod narusza zasadę YAGNI. |
|
karambaHZP |
» 2016-07-01 10:51:52 | Ale wtedy z kolei kod narusza zasadę YAGNI. |
Może jest to pisanie na wyrost, ale kod zyska na czytelności (jak zastosowanie algorytmów - michal11). Przy sensownych nazwach, nie trzeba analizować całej funkcji, aby zrozumieć czym się zajmuje. Rozbijanie kodu na pojedyncze funkcjonalności uważam za dobrą praktykę. Co do przepychania się na zasady, nie ma kodu idealnego. Przed napisaniem każdej linii warto zastanowić się, czy będę rozumiał ten kod, gdy zajrzę do niego za kilka miesięcy. void usunSasiadujaceDuplikaty( string & napis, char szukanyDuplikat )
{
size_t pos = 0;
if( szukanyDuplikat == ' ' ) {
while( napis[ 0 ] == szukanyDuplikat ) {
napis.erase( 0, 1 );
}
}
while(( pos = napis.find( ' ', pos ) ) &&( pos != string::npos ) ) {
if( pos < napis.size() - 1 ) {
if( napis[ pos + 1 ] == ' ' ) {
napis.erase( pos, 1 );
}
else {
++pos;
}
}
}
}
void podmienZnaki( string & napis, char znakDoPodmiany, char nowyZnak )
{
for( char & element: napis ) {
if( element == znakDoPodmiany )
element = nowyZnak;
}
}
string konwertuj( string & napis )
{
string rezultat { napis };
char szukanyDuplikat { ' ' };
char znakDoPodmiany { '<' };
char znakDoPodmiany2 { '>' };
char nowyZnak { '[' };
char nowyZnak2 { ']' };
usunSasiadujaceDuplikaty( rezultat, szukanyDuplikat );
podmienZnaki( rezultat, znakDoPodmiany, nowyZnak );
podmienZnaki( rezultat, znakDoPodmiany2, nowyZnak2 );
return rezultat;
}
int main()
{
string oryginalnyNapis { "<b>to jest </b> testowy napis <b>:)" };
cout << oryginalnyNapis << '\n';
cout << konwertuj( oryginalnyNapis ) << endl;
}
Teraz trzeba odpowiedzieć na pytania. Czy to jest nadmierne komplikowanie kodu? Czy zyskujemy na zrozumieniu kodu? Czy ewentualny błąd łatwiej naprawić w jednej małej funkcji, niż szukanie w dużej. Lub: string konwertuj( string & napis )
{
string rezultat { napis };
char szukanyDuplikat { ' ' };
char znakDoPodmiany { '<' };
char znakDoPodmiany2 { '>' };
char nowyZnak { '[' };
char nowyZnak2 { ']' };
if( szukanyDuplikat == ' ' ) {
while( rezultat[ 0 ] == szukanyDuplikat ) {
rezultat.erase( 0, 1 );
}
}
size_t pos = 0;
while( pos <( rezultat.size() - 1 ) ) {
if( rezultat[ pos ] == szukanyDuplikat && rezultat[ pos + 1 ] == szukanyDuplikat ) {
rezultat.erase( pos, 1 );
}
else if( rezultat[ pos ] == znakDoPodmiany ) {
rezultat[ pos ] = nowyZnak;
++pos;
}
else if( rezultat[ pos ] == znakDoPodmiany2 ) {
rezultat[ pos ] = nowyZnak2;
++pos;
}
else {
++pos;
}
}
return rezultat;
}
Czy zaglądając za kilka miesięcy, będę wiedział o co tu chodzi? Czy jednak będę musiał dokładnie przeanalizować całą funkcję? |
|
Szkaplerny Temat założony przez niniejszego użytkownika |
» 2016-07-03 22:34:36 Dzięki za konstruktywną dyskusję i wskazanie błędów.
@michal11 - co do formatowania - takie rozwiązanie wyrobił u mnie VS, nie zmieniałem tego, i jakoś się przyzwyczaiłem, że nawet jest czytelnie, jak dla mnie ofc.
@karambaHZP - zupełnie zapomniałem o zadawaniu samemu sobie pytań, tak jak to zrobiłeś po każdym przykładzie i dzięki za ich okomentowanie. |
|
Elaine |
» 2016-07-04 09:06:47 Większość podanych tu rozwiązań ma poważny problem: są niepotrzebnie kwadratowe. Da się to zrobić w czasie liniowym, choćby i przez std::replace i std::unique. Gdyby koniecznie miało to być zrobione w jednej pętli, to pewnie zrobiłbym tak: std::string konwertuj( std::string string )
{
std::string::size_type dest_index { 0 };
for( std::string::size_type i { 0 }; i < string.size(); ++i ) {
const char current_char { string[ i ] };
if( current_char == ' ' && string[ i + 1 ] == ' ' ) {
continue;
}
switch( current_char ) {
case '[': string[ dest_index ] = '<'; break;
case ']': string[ dest_index ] = '>'; break;
default: string[ dest_index ] = current_char; break;
}
++dest_index;
}
string.resize( dest_index );
return string;
} | @michal11 - co do formatowania - takie rozwiązanie wyrobił u mnie VS, nie zmieniałem tego, i jakoś się przyzwyczaiłem, że nawet jest czytelnie, jak dla mnie ofc. |
Bo jest czytelne. Każdy przecież wie, że 1TBS > Allman. |
|
| 1 « 2 » |