Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Transform hand written loop to std library call

Tags:

c++

c++11

c++14

I have recently watched the talk from Sean Parent about C++ seasoning from 2013. If I understood him right, he says that you can eliminate almost all (all?) hand written loops from your code. My question is how to achieve this? Let's consider the following code:

class ProgressDialog
{
  //interesting part of that class
  void SetPosition(int position);
  bool IsCancelRequested();
  void SetHeader(const std::string& status);
}
void foo( const std::vector<std::string>& v)
{
  ProgressDialog dlg;
  long position = 0;
  for( const auto& s : v)
  {
    ++position;
    dlg.SetPosition(position);
    dlg.SetHeader("Processing"+ s);
    DoSomethingThatTakesSomeTime(s);
    if(dlg.IsCancelRequested()) break;
  }
}

Is there a way to refactor away the hand written loop?

like image 553
Marc Avatar asked Oct 02 '18 07:10

Marc


2 Answers

I'm not sure if this adds any clarity, but it is an attempt to refactor the concept of the loop and early break.

#include <string>
#include <vector>
#include <ciso646>

struct ProgressDialog
{
  //interesting part of that class
  void SetPosition(int position);
  bool IsCancelRequested();
  void SetHeader(const std::string& status);
};
void DoSomethingThatTakesSomeTime(std::string const&);

// iterate over a container, calling func with the current value.
// if func returns false, cease iterating immediately.
// return true if terminated early
// note: func must return false if it wishes early termination,
// otherwise true
template<class Cont, class F> 
auto for_each_while(Cont&& container, F&& func)
{
    for(auto&& s : container)
        if (not func(s)) 
            return true;
    return false;
}

void foo( const std::vector<std::string>& v)
{
    auto update_dialog = 
    [position = 0, dlg = ProgressDialog()](auto&& s) mutable
    {
        ++position;
        dlg.SetPosition(position);
        dlg.SetHeader("Processing"+ s);
        DoSomethingThatTakesSomeTime(s);
        return !dlg.IsCancelRequested();
    };
    for_each_while(v, update_dialog);
}

And here is some std library abuse which achieves the same thing.

I strongly suggest that you do not do this, as it is not clear to the casual reader what is going on!

void foo( const std::vector<std::string>& v)
{
    int position = 0;
    auto dlg = ProgressDialog();

    std::find_if(begin(v), end(v), 
                 [&](auto&& s)
    {
        ++position;
        dlg.SetPosition(position);
        dlg.SetHeader("Processing"+ s);
        DoSomethingThatTakesSomeTime(s);
        return dlg.IsCancelRequested();
    });
}
like image 91
Richard Hodges Avatar answered Nov 07 '22 15:11

Richard Hodges


The general case

If I understood him right, he says that you can eliminate almost all (all?) hand written loops from your code.

Well, yes and no - obviously it depends on what kind of loops you write. He slightly exaggerated, I believe, to emphasize the fact that many loops are not really necessary, and may better be refactored into application of standard library "algorithms".

Also, essentially any loop can be replaced with an std::for_each, but that doesn't really count since it only hides the explicit loop progression control and is still "just a loop".

Your specific example

In your case, the loop iteration does work while logging/reporting each iteration of the work being done, and being willing to accept an abort request. You could consider writing a variant of std::for_each with that additional aspect of functionality, then using it for the actual work as a special case, e.g.

namespace with_progress_dialog {

template< class InputIt, class UnaryFunction >
void for_each(
    InputIt          first, 
    InputIt          last,
    UnaryFunction    function,
    std::string_view message_prefix = "Processing item " )
{
    ProgressDialog progress_dialog;
    for (position_t position = 0; first != last; ++first, ++position) 
    {
        progress_dialog.SetPosition(position);
        progress_dialog.SetHeader(message_prefix + position);
        function(*first);
        if ( progress_dialog.IsCancelRequested() ) { break; }
    }
}
} 

and then call

// ... etc. etc. ...
with_progress_dialog::for_each(
    std::begin(v), std::end(v),
    &DoSomethingThatTakesSomeTime);

Now, this is somewhat of an over-generalization, for sure. But I'm assuming you have other cases in which you open a progress dialog that gradually updates. So perhaps adapt your generalization accordingly. Or - maybe you can keep some sort of a window-level state when you're doing heavy lifting, and have another thread track that and open a dialog window when necessary, or show an indication on a status bar. Yet another option maybe to have that dialog run in a co-rountine (but that's pretty speculative, not sure it's a good idea).


Note: It is not a good idea, and potentially unsafe, to just go ahead and print strings you're working on - which can likely originate in inputs you don't control. You need to make sure they're not too long, and consider sanitizing them. In my code I'm only printing the index ("position").

like image 5
einpoklum Avatar answered Nov 07 '22 17:11

einpoklum