1

Task Description

I need a way to wrap different types of progress bars in a way that an algorithm can update the progress without knowing about the exact implementation. The progress implementation may allow the user to request to cancel the operation and the algorithm can query if it should stop at points where it can safely abort the current operation.

Question

Now I am looking for how to design a clean API, that does not require backward incompatible changes in the future.

I need these functions:

  • Start a task with an optional description.
  • Increase the progress.
  • Enable or disable a cancel button.
  • Communicate cancel events to the algorithm, that should stop as soon as possible.
  • Finish the progress, e.g., by hiding the progress bar.
  • Possibly update the task description between different steps of the algorithm.
  • Maybe output some logging information that should be displayed in the UI (e.g. "rounding error occured, the result may be inaccurate")

Why decide on a final API now?

I am breaking up a large code base by creating smaller libraries and I want to open source some of the functions that may be useful for others. For most functions the API is quite clear and will not need larger changes later on and such changes may provide backward compatibility by using overloaded functions.

But when trying to find an implementation for a generic Progress object, I see problems when I need to add a function later that needs to be added to the algorithms or implemented by the progress implementations. To avoid trouble for other users, I look for an API that will not need to be changed later on.

Restrictions

As I do not implement (all) the progress bar implementations myself, I have the restriction that I cannot require them to use callbacks for the cancel operation.
In fact I need to wrap one API (Maya progressBar) that uses a isCancelled method that needs to be called in order to determine if the user clicked the cancel button.

How the interface could look like

An implementation with a progress bar (GUI or text UI) may set the maximum value to 50 and display a cancel button when canStop() is true.

[====      ] 40% [cancel]

(text representation of a GUI)

A text mode implementation may calculate the percentage and display a simple progress bar, but ignore canStop()

0 --------------- 100
  |||||

(actual text output)

Current Implementation

This is my current attempt for the API (updated with some feedback from codereview.stackexchange.com):

class Progress {
public:
    virtual ~Progress() {};
    virtual void start(std::string description, uint maxProgress, uint initial_progress, bool can_stop) = 0;
    virtual void end() = 0;
    virtual void progress(int value) = 0;
    virtual void incProgress(int steps = 1) = 0;
    virtual bool shouldStop() const = 0;
    virtual bool wasStopped() const = 0;
};

Here I already dropped the info(std::string message) method, as it probably should not belong the progress implementation.

Example Implementation

This is an example implementation for a simple text output, that does not support the cancel operation.

class SimpleProgress: public Progress {
public:
    SimpleProgress(int progress_bar_length = 50): progress_bar_length(progress_bar_length) {};

    virtual void start(std::string description, uint max_progress = 0, uint initial_progress = 0, bool can_stop = false) override {
        std::cerr << "Starting: " << description << std::endl;
        this->description = description;
        this->max_progress = max_progress;
        this->current_progress = initial_progress;
    };

    virtual void end() override {
        progress(max_progress);
        std::cerr << "Finished: " << description << std::endl;
    };

    virtual void progress(int progress) override {
        assert(progress >= 0);
        assert(progress <= max_progress);
        for(int i = 0; i < (progress - current_progress) / max_progress * progress_bar_length; i++) {
            std::cerr << "=";
        }
        current_progress = progress;
    };

    virtual void incProgress(int steps = 1) override {
        assert(steps > 0);
        if(max_progress >= 0) {
            assert(current_progress < max_progress);
            progress(current_progress + steps);
        } else {
            // Progress without a known maximum value.
            std::cerr << "." << std::endl;
        }
    };

    virtual bool shouldStop() const override {
        return false;
    }

private:
    int current_progress = 0;
    int max_progress = -1;
    int progress_bar_length;
    std::string description;
};

For a non-interactive implementation, you could for example want to stop when a certain runtime is exceeded, implementing the methods like this:

        virtual void setCanStop(bool can_stop) {
                _can_stop = can_stop;
        };

        virtual bool canStop() {
                return _can_stop;
        };

        virtual bool shouldStop() {
                if(_can_stop && runtime >= max_runtime) {
                        return true;
                } else {
                        return false;
                }
        }

Example Usage

This is how I would use the Progress object:

void doSomething(Progress *progress = nullptr) {
    DummyProgress dummyProgress;
    if(progress == nullptr) {
        progress = &dummyProgress;
    }
    // progress bar for a 50 step operation that can be stopped.
    progress->start("Calculating something", 50, 0, true);
    for(int i=0; i < 50; i++) {
        progress->progress(i);
        // Calculate something for step i
        if(progress->shouldStop()) {
            progress->end();
            break; // stop the calculcation
        }
    }
    if(progress->wasStopped()) {
        std::cerr << "Not all items were processed." << std::endl;
    } else {
        progress->end();
    }
}

Note: I originally posted this question to Code Review and they recommended to ask such more theoreic questions about good APIs here.

6
  • If I understand it correctly, the interface requires that you know the number of steps beforehand (max_progress). However, there are algorithms where you initially do not know the number of steps, for example, path finding. Do you want to take these into account or would it be okay to ignore such algorithms for now?
    – pschill
    Mar 11, 2020 at 13:39
  • Will progresses always succeed or is it possible that they can fail? In that case you want to think about communicating the failure, for example, using a success-flag and an error description.
    – pschill
    Mar 11, 2020 at 13:42
  • @pschill I may not know the steps. A text progress bar could for example output one dot per step, see the example code. A ui progress bar probably would just keep at 0 until end is called and possibly show a counter "x items processed".
    – allo
    Mar 11, 2020 at 15:28
  • @pschill The progress may fail, but I think handling this is up to the program that uses the algorithm. I want to add a generic Progress interface to a library that can be used in all sorts of programs. The program would implement the UI and wrap it in a Progress subclass. Then it starts the algorithm and waits for it to stop (completed, failed or aborted). Abortion should be indicated by the wasStopped method. Failure should be indicated by return value or other mechanisms that are unrelated to the Progress object.
    – allo
    Mar 11, 2020 at 15:31
  • 1
    Okay that should be fine. There is an error in the SimpleProgress example that may cause headaches later: The progress() function asserts max_progress >= 0, however it divides by max_progress, which can be a division by zero.
    – pschill
    Mar 11, 2020 at 15:51

3 Answers 3

3

As with every API, naming is key. Good names support documentation and prevent invalid use.

From the Progress class itself, it is not clear per se what the class is for and what the responsibilities are. A better name would be IProgressReporter (following the guideline to prefix interfaces with an 'I') which should nmake clear that any implementations of this interface are meant to report some progress information.

The method names start() and end() may be confusing. These names suggest that something is started/stopped as a side effect when the mehods are called. You should rename them to setStarted() and setStopped().

The name of the method progress() does also not convey enough information. A better name would be setProgress().

Also, the method shouldStop() should be renamed to wasAbortRequested() to make things more clear.

With the method wasStopped(), I would suggest removing it from the interface, checking if the task was in fact aborted should not be done via this interface.

In terms of funtionality, I would enhance the setProgress() and incProgress() methods so that you can pass a message string and pass strings as const references instead of copies. You may add overloads without progress message if that is more suitable.

Your interface would then look like this:

class IProgressReporter 
{
public:
    virtual ~IProgressReporter() {};
    virtual void setStarted(const std::string& description, uint maxProgress, 
        uint initial_progress, bool canAbort) = 0;
    virtual void setStopped() = 0;
    virtual void setProgress(const std::string& message, int value) = 0;
    virtual void incProgress(const std::string& message, int steps = 1) = 0;
    virtual bool wasAbortRequested() const = 0;
};
2
  • Thank you for the feedback. I am not sure about started and stopped, because I think a method name should be an action and not a state. Started sounds a bit like a shorter "isStarted". I agree with the wasAbortRequested, because |shouldStop| could mean something like "setShouldStop". Using const ref arguments is a good idea as well.
    – allo
    Mar 11, 2020 at 15:36
  • 1
    @allo You are right, these names aren't concise either. I updated my answer -> names are now setStarted and setStopped.
    – helb
    Mar 11, 2020 at 15:40
1

In my opinion, you are seriously over-thinking this thing! You need:

  • Show/hide the progress bar.

  • Set the percentage value of the progress bar. (0-100.)

  • Enable or disable the cancel button.

  • Poll to determine if the button has been clicked.

  • Maybe let the programmer provide a closure that will be called whenever the button is clicked.

The process that will be using this API will be responsible for setting the value of the bar and for periodically checking to see if the user wants to cancel. It's also responsible for actually stopping. None of these things are the concern of your implementation.

As I heard it said once, "don't design a beautiful archway to put over the front door of a supermarket."

3
  • The point why I want to do it right the first time is, that I want to split a larger code base in smaller parts that can work independent from each other. But this also means, that changing an API function in the progress object may require changing both the algorithms library and the programs using the algorithms.
    – allo
    Mar 11, 2020 at 15:48
  • @allo - Perhaps trying to "do it right the first time" is the wrong approach; you'll be in a bigger mess if you introduce wrong abstractions too early. It's probably better to do something simpler that works now, and then keep refactoring/restructuring for some time, as the need arises (if it arises at all), until the API stabilizes (until the right abstractions "emerge", so that the most likely kinds changes are easily supported) - then you'll end with decoupled & cohesive components. Mar 11, 2020 at 19:48
  • @FilipMilovanović My problem is, that I would like to put some algorithms in an open source library that others may find useful. I could refactor my projects in a reasonable time, but for others it will be a surprise when an update changes the API. So my goal is to make sure I do not need to break the API for them.
    – allo
    Mar 12, 2020 at 14:08
-3

I'd now like to specifically draw attention to what Allo said ...

The point why I want to do it right the first time is, that I want to split a larger code base in smaller parts that can work independent from each other. But this also means, that changing an API function in the progress object may require changing both the algorithms library and the programs using the algorithms.

Whoa!

IMHO, if indeed it is true that "[doing something as apparently innocent as ...] changing an API function in the progress object" ... might require "changing the algorithms library" and(!!) "the programs using the algorithms [library]" ...

FULL STOP!

"Goody for you that you wanna do it right." Who! Cares!

This change of yours very obviously has a substantial "downstream ripple," and for what? For a progress bar?! Because one lonely programmer decided to "do it right, according to that one lonely programmer?!"

Really, the most important thing about any "software that right-now works" is that "right now, it works." Maybe it doesn't do it in the most optimal or least odiferous way, and maybe we'd never do it that way ever again, but ... it works.

"Seemingly innocent" API-changes such as this one can actually turn out to be profoundly disruptive because they "ripple out" to an unforeseeable number of other, otherwise-unrelated systems, demanding simultaneous changes to be made to each and every other one of those systems ("And may God help you if you miss a single one!") ... and for what?

1
  • 1
    I find it hard to follow your rather emotional style of writing and I am not sure what your actual answer or suggestion is.
    – allo
    Mar 12, 2020 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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