Który zapis jest dla was czytelniejszy?

0

Zapis nr.1:

std::string strNumbers = "1,2"; //arg funkcji
std::vector<int> vOfNumbers;
for (const auto& it : strNumbers) {
    if (isdigit(it))
        vOfNumbers.push_back(it - '0');
}
return std::accumulate(std::begin(vOfNumbers), std::end(vOfNumbers), 0);

Zapis nr.2:

std::string strNumbers = "1,2"; //arg funkcji
std::vector<int> vOfNumbers;
for_each(std::begin(strNumbers), std::end(strNumbers),
    [&vOfNumbers](const auto& it) {
        if (isdigit(it))
            vOfNumbers.push_back(it - '0');
    });
return std::accumulate(std::begin(vOfNumbers), std::end(vOfNumbers), 0);

Cześć, mam takie pytanie: Wolicie czytać coś "prostego" jak zapis nr.1 czy to bez znaczenia dla was i zapis nr. 2 wygląda tak samo? Jeśli załóżmy rozwijamy algorytm, który jest dość nieczytelny przez stl, to lepiej napisać to w prostszy sposób czy zostawić takiego krzaka?

tbh wiem, że głupi wątek, ale nie znam nikogo, kto by zrobił mi code review, albo przynajmniej powiedział, że coś rozumie z kodu, a więc polegam na tym forum.
Pozdro!

5

for_each miał sens w czasach, gdy nie było range for loop.
Sam algorytm jest dziwnie pomyślany.
IMO było by lepiej (C++17):

int sumDigitsInStr(std::string_view s) {
    int sum = 0;
    for(auto ch : s) {
         sum += std::isdigit(ch) ? ch - '0' : 0;
    }
    return sum;
}

albo:

int sumDigitsInStr(std::string_view s) {
    return std::accumulate(std::begin(s), std::end(s), 0, 
        [](auto sum, auto ch) { 
            return sum + (std::isdigit(ch) ? ch - '0' : 0);
        });
}
0

@MarekR22: dzięki, drugie rozwiązanie naprawdę fajne. Też próbowałem kombinować z ternary operator, ale nie wiedziałem właśnie co zrobić w przypadku fałszu więc zrezygnowałem. Kurdę ten std::string_view mnie dezorientuje trochę. Prawie jest taki sam jak std::string. Jeśli na produkcji bym napisał algorytm jednym z powyższych sposobów, to jaka byłaby reakcja teamu? :D

0
Cyberah napisał(a):

Jeśli na produkcji bym napisał algorytm jednym z powyższych sposobów, to jaka byłaby reakcja teamu? :D

Pewnie coś w stylu "Debilu! Nie wrzucaj nieprzetestowanego kodu na produkcję!"

0

:D akurat to zadanie jest tworzone w parze z TDD, to chyba byłbym uratowany :P

1
Cyberah napisał(a):

Kurdę ten std::string_view mnie dezorientuje trochę. Prawie jest taki sam jak std::string. Jeśli na produkcji bym napisał algorytm jednym z powyższych sposobów, to jaka byłaby reakcja teamu? :D

std::string_view to taki wskaźnik/referencja na dowolny napis lub jego wycinek.
Jeśli wywołasz tą funkcję tak: sumDigitsInStr("jakis napis") to tymczasowy std::string nie zostanie utworzony, co jest sporą zaletą.
Tak samo sumDigitsInStr(jakisStdString) też zadziała, bez narzutów.

6

Wersja C++20 też jest spoko:

int sumDigitsInStr(std::string_view s) {
    using namespace ranges::views;
    return ranges::accumulate(s | filter([](auto ch){ return std::isdigit(ch); })
                                | transform([](auto ch){ return ch - '0'; }),
                              0);
}

https://wandbox.org/permlink/iRYwnyeHxEwir4DO

1 użytkowników online, w tym zalogowanych: 0, gości: 1