5
\$\begingroup\$

I want to change the legacy C-style code by using stl:

for (posEmptyItem = startAt; strlen(collection[posEmptyItem]) > 10; posEmptyItem++) {}
std::cout << posEmptyItem << std::endl;

This code seems a bit hard to read. Anyway to do better?

auto it = std::find_if(collection + startAt, 
                       collection + COLLECTION_SIZE, 
                       [](const char* line) { return strlen(line) <= 10; });
int idx = std::distance(collection, it); 

Below a complete example:

#include <cstring>
#include <iostream>
#include <algorithm>

#define COLLECTION_SIZE      250

int main()
{
    const char* collection[COLLECTION_SIZE]{ "time11,time2,time3",
                                       "time12,time2,time3",
                                       "time13,time2,time3",
                                       "time14,time2,time3",
                                       "time15,time2,time3",
                                       "x\n", 
                                       "" };
    auto startAt = 2;
    int posEmptyItem;

    // legacy code
    for (posEmptyItem = startAt; strlen(collection[posEmptyItem]) > 10; posEmptyItem++) {}
    std::cout << posEmptyItem << std::endl;

    // replace the loop to search an index by calling to standard library
    auto it = std::find_if(collection + startAt, 
                           collection + COLLECTION_SIZE, 
                           [](const char* line) { return strlen(line) <= 10; });
    posEmptyItem = std::distance(collection, it); 
    std::cout << posEmptyItem << std::endl;

    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ "This code seems a bit hard to read." Why do you believe it's hard to read actually. \$\endgroup\$ – πάντα ῥεῖ May 11 '17 at 18:22
  • \$\begingroup\$ To compute the offset from an iterator looks stange to me. \$\endgroup\$ – Less White May 11 '17 at 18:25
  • \$\begingroup\$ There's nothing weird about using std::distance (I don't get where you did getting std::dist from. Maybe a MSVC specific typedef??) \$\endgroup\$ – πάντα ῥεῖ May 11 '17 at 21:35
4
\$\begingroup\$

For "easier readability" you could extern the lambda expression form the find_if():

auto pred = [](const char* line) { return strlen(line) <= 10; };
auto it = std::find_if(collection + startAt, 
                       collection + COLLECTION_SIZE,
                       pred);

Also make use of std::begin() and std::end():

auto it = std::find_if(std::begin(collection) + startAt, 
                       std::end(collection),
                       pred);

At least (but probably not last), don't use raw arrays. Rather change collection to a std::array:

std::array<const char*,COLLECTION_SIZE> collection 
    { "time11,time2,time3"
    , "time12,time2,time3"
    , "time13,time2,time3"
    , "time14,time2,time3"
    , "time15,time2,time3"
    , "x\n"
    , "" 
    };
// Note my formatting style above, which makes it easier to extend the array

Here's the fully refactored code:

#include <cstring>
#include <iostream>
#include <algorithm>
#include <array>

const size_t COLLECTION_SIZE = 250; // rather use a const variable than a macro

int main()
{
    std::array<const char*,COLLECTION_SIZE> collection
        { "time11,time2,time3"
        , "time12,time2,time3"
        , "time13,time2,time3"
        , "time14,time2,time3"
        , "time15,time2,time3"
        , "x\n"
        , "" 
    };

    size_t startAt = 2; // care about the correct type. auto would leave you with int

    // replace the loop to search an index by calling to standard library
    auto pred = [](const char* line) { return strlen(line) <= 10; };
    auto it = std::find_if(std::begin(collection) + startAt, 
                           std::end(collection), 
                           pred);
    auto posEmptyItem = std::distance(std::begin(collection), it);
    std::cout << posEmptyItem << std::endl;

    return 0;
}

See Live Demo

\$\endgroup\$
  • \$\begingroup\$ Thanks for your help. I am learning the standard library and want to be check that my code is ok. \$\endgroup\$ – Less White May 11 '17 at 18:28
  • \$\begingroup\$ @LessWhite if you find the answer helpful you can upvote it ;-) \$\endgroup\$ – t3chb0t May 11 '17 at 18:35
  • \$\begingroup\$ I don't think that std::array is a good substitute. No type traits work on it, even (ironically) std::is_array. \$\endgroup\$ – Incomputable May 11 '17 at 20:16
  • \$\begingroup\$ @Incomputable Feel free to make a better proposal for that particular case. \$\endgroup\$ – πάντα ῥεῖ May 11 '17 at 20:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.