HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Retrieve the index of the first element using a predicate

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thepredicateretrievefirstelementusingindex

Problem

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 
#include 
#include 

#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;
}

Solution

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 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 
#include 
#include 
#include 

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

int main()
{
    std::array 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

Code Snippets

auto pred = [](const char* line) { return strlen(line) <= 10; };
auto it = std::find_if(collection + startAt, 
                       collection + COLLECTION_SIZE,
                       pred);
auto it = std::find_if(std::begin(collection) + startAt, 
                       std::end(collection),
                       pred);
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
#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;
}

Context

StackExchange Code Review Q#163099, answer score: 4

Revisions (0)

No revisions yet.