4
\$\begingroup\$

Now, I have this text UI progress bar for terminals that looks like:

[##########################################################################....]

Code

package io.github.coderodde.textui.components;

/**
 * This class implements a simple text UI progress bar.
 */
public class TextProgressBar {

    private float progress = 0.0f;
    private final int width;
    private boolean printedOnce = false;
    
    public TextProgressBar(int width) {
        this.width = checkWidth(width);
    }
    
    public void print() {
        if (printedOnce) {
            clearBar();
        } else {
            printedOnce = true;
        }
        
        System.out.print(getBar());
        System.out.flush();
    }
    
    public void inc(float dx) {
        progress += dx;
        progress = (float) Math.max(0.0, Math.min(1.0, progress));
    }
    
    private void clearBar() {
        for (int i = 0; i < width + 2; ++i) {
            System.out.print("\b");
        }
    }
    
    public static void main(String[] args) {
        TextProgressBar bar = new TextProgressBar(78);
        
        for (float f = 0.0f; f <= 1.005f; f += 0.01f) {
            bar.print();
            bar.inc(0.01f);
            
            try {
                Thread.sleep(100L);
            } catch (InterruptedException ignored) {
                
            }
        }
    }
    
    private String getBar() {
        int nFilled = (int)(progress * width);
        int nNonFilled = width - nFilled;
        
        StringBuilder sb = new StringBuilder(width + 2).append("[");
        
        for (int i = 0; i < nFilled; ++i) {
            sb.append("#");
        }
        
        for (int i = 0; i < nNonFilled; ++i) {
            sb.append(".");
        }
        
        return sb.append("]").toString();
    }
    
    private static int checkWidth(int width) {
        if (width < 1) {
            throw new IllegalArgumentException(
                String.format("width (%d) < 1", width));
        }
        
        return width;
    }
}

Critique request

Please, tell me anything that comes to mind.

NB

NetBeans refused to erase the previous progress bar on the same line, so it doesn't work in all environments.

\$\endgroup\$
3
  • \$\begingroup\$ bar.setProgress(f) would seem more sensible than bar.inc(0.1f) - do not get the two loop counters out of sync \$\endgroup\$ Commented Jul 5 at 23:18
  • \$\begingroup\$ Is StringBuilder reuse worth a try? The class is IO-bound and is already thread-unsafe, so it should not break anything. On the other hand, we should try to avoid unnecessary state. \$\endgroup\$ Commented Jul 6 at 13:10
  • \$\begingroup\$ It would be nice not to suppress InterruptedException. This is a demo, yes, but throwing it from main() is even shorter. \$\endgroup\$ Commented Jul 6 at 13:12

4 Answers 4

6
\$\begingroup\$

Clients should not care about throttling

I believe separation of print() and inc() methods leaks abstraction. It makes caller responsible for print throttling/scheduling.

As throttling is an essential function of progress reporting, I suggest to future-proof the API by combining these methods in a single inc(), moving throttling responsibility from clients to TextProgressBar. Then, when it is time to introduce a complicated throttling, the changes would not affect TextProgressBar clients.

Meanwhile, ensure that no IO operations are done when progress report bar is unchanged: only report when dx accumulated since last report is greater than 1/width.

Do not use System.out

Standard output can not be used for information that is not considered to be your program's "final" product. You should be able to pipe your standard output to a file and get sensible, reusable results. This would be impossible if results are mixed with progress reporting. So you have to use another available stream - System.err for diagnostic or intermediate outputs including progress reporting. Or inject a desired stream.

Width is a lie

A canonical way to construct an object of your class is:

import org.jline.terminal.TerminalBuilder;
var progress = new TextProgressBar(TerminalBuilder.terminal().getWidth());

But that would cause progress bar to overflow and take two lines:

[########....
.]

Documenting a constructor argument to be "at most available width - 2" would leak implementation detail (a margin of size 2). Therefore, your constructor should take care to reduce the given size by two and actually accept total available width (normally terminal width).

An ideal progress bar queries terminal width before every redraw (because terminal windows can be resized dynamically) and constructor argument could be avoided or replaced with IntSuppler, but this approach introduces more dependencies or complexity, and hence is optional.

\$\endgroup\$
6
\$\begingroup\$
        progress = (float) Math.max(0.0, Math.min(1.0, progress));

Since Java 21, we have Math.clamp() for that (with overloads for int, long, float and double):

        progress = Math.clamp(progress, 0.0f, 1.0f);
\$\endgroup\$
4
\$\begingroup\$

Further commenting on the but if code Toby Speight has noted, incrementing progress and then immediately assigning to it should raise questions. Better to just:

    public void inc(float dx) {
        progress = (float) Math.max(0.0, Math.min(1.0, progress + dx));
    }
\$\endgroup\$
4
\$\begingroup\$

It would be extremely rare for a progress bar to go naturally from 0 to 1.0f. Usually a progress bar goes from 0 to 26084276 bytes. So, instead of requiring the user to do boring math every time they use the component, construct the progress bar with the target result, increase towards that target and do the math that converts "26084276 bytes" to "100 %" inside the component.

A good way to figure out what the users would like to see in an API is to use it a bit before making it public and then fix the things you found difficult.

\$\endgroup\$
3
  • \$\begingroup\$ While I do see this in real libraries, they are quite inconvenient to use, as they require to determine the total amount of work to initialize progress bar. This forces unnatural flows, where a bunch of computation happens before common initialization steps. This is particularly annoying when initialization is required to determine the total work and there are many steps to it. \$\endgroup\$ Commented Jul 7 at 8:17
  • 1
    \$\begingroup\$ Then you can add the possibility to modify the target after initialization. The common use case still is that the user knows an arbitrary total and the accumulated amount and the conversion is always the same. So it's better to not require the user to do it if they don't want. \$\endgroup\$ Commented Jul 7 at 9:32
  • \$\begingroup\$ The user can still choose a target of 1.0 if desired - so existing code using this class needs no change. The option to choose the scale helps simplify calling code by letting it work in its natural units,. \$\endgroup\$ Commented Jul 7 at 12:34

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.