Console Application Customization - GetBootstrap v2.8.6.19

Question

It's been a long time since my last post. I decided to continue this little project and tried to debug the GetBootstrap v2.7. I found out that the progress bar that I create is not working properly when the size of console buffer is changes. so I decide to rewrite it again.

Below, the progress task is to rewrite itself from the last call of ProgressBar.Write() and Increment() its value. I also use thread lock to prevent other Console.Write() to overlap each other. Is there any better way to improve this code? By the way, you can download the source code in GetBootstrap v2.8.6.19 to have a better view.

enter image description here

ProgressBar.cs

namespace System.Extensions
{
 public class ProgressBar
 {
 private int _cwidth;
 private int _cheight;
 private int _csrtop;
 private int _csrleft;
 private int _progwidth;
 private int _value = 0;
 private int _maxvalue;
 private float _width = 50;
 private bool _showPercent = true;
 private bool _showSeparator = true;
 public int Value { get => _value; set => _value = value; }
 public int MaxValue { get => _maxvalue; set => _maxvalue = value; }
 public float Width
 {
 get => _width; set
 {
 if (value > 100)
 {
 throw new ArgumentOutOfRangeException("Width must not greater than to 100");
 }
 else if (value < 0)
 {
 throw new ArgumentOutOfRangeException("Width must not less than to 0");
 }
 _width = value;
 }
 }
 public ConsoleColor BackgroundColor { get; set; } = ConsoleColor.Gray;
 public ConsoleColor ProgressColor { get; set; } = ConsoleColor.Cyan;
 public ConsoleColor SeparatorColor { get; set; } = ConsoleColor.Gray;
 public bool ShowPercent { get => _showPercent; set => _showPercent = value; }
 public bool ShowSeparator { get => _showSeparator; set => _showSeparator = value; }
 public ProgressBar(int maxValue = 100)
 {
 _maxvalue = maxValue;
 }
 public void Write()
 {
 _cwidth = (int)(((Console.BufferWidth - (_showPercent ? 7 : 0)) / 100f) * _width);
 _cheight = Console.BufferHeight;
 _csrtop = Console.CursorTop;
 _csrleft = Console.CursorLeft;
 _progwidth = 0;
 for (int i = 0; i < _cwidth - _csrleft; i++)
 {
 Console.BackgroundColor = BackgroundColor;
 Console.ForegroundColor = SeparatorColor;
 Console.Write(_showSeparator ? "_" : " ");
 _progwidth++;
 }
 }
 public void WriteLine()
 {
 Write();
 Console.WriteLine();
 }
 public void Increment(int increment = 1)
 {
 lock (Bootstrap._threads)
 {
 _value += increment;
 if (_value <= _maxvalue)
 {
 int _cursortop = Console.CursorTop;
 int _cursorleft = Console.CursorLeft;
 Console.CursorTop = _csrtop;
 Console.CursorLeft = _csrleft;
 Console.BackgroundColor = ProgressColor;
 float progress = (_value / (float)_maxvalue) * 100f;
 float pbwidth = (_progwidth / 100f) * progress;
 for (int i = 0; i < pbwidth; i++)
 {
 Console.ForegroundColor = SeparatorColor;
 Console.Write(_showSeparator ? "_" : " ");
 }
 Console.ResetColor();
 if (_showPercent)
 {
 Console.CursorLeft = _cwidth;
 Console.WriteLine($" {progress.ToString("0.00")}%");
 }
 Console.CursorTop = _cursortop;
 Console.CursorLeft = _cursorleft;
 }
 }
 }
 }
}

• Raimund Krämer: Grammar in the exception messages: "Width must not greater than to 100" should be "Width must not be greater than 100", and "Width must not less than to 0" should be "Width must not be less than 0".
• t3chb0t: Strangely your code was better in the previous version 2.7 three years ago than it is now.
• Onel Sarmiento: @t3chb0t Maybe because I used a lot of private fields, someone told me that I should not give direct access to a member variables and use properties for accessing it instead.

Answer 1

Bugs

There's a fair amount of bugs in your code. The 'happy path' works, but it's easy to break something. Especially when you're writing library-level code it's useful to occasionally look at your code with a 'how can I break this' mindset.

• Write and WriteLine only draw an empty progress bar, without percentage label, even when Value is higher than zero.
• Increment won't draw the progress bar if Value is larger than MaxValue. Use Math.Min(Value, MaxValue) instead of an if statement.
• Neither Value's setter nor Increment protect Value from becoming negative, so you can end up with a negative percentage. In some cases that can also cause the percentage label to wrap around to the next line.
• Setting MaxValue to 0 results in NaN% or in some cases -∞%. You should either enforce that MaxValue must be larger than the minimum value (0), or take these edge-cases into account in your redraw logic.
• Calling Increment without having called Write or WriteLine does not draw a progress bar, only a percentage label.
• Calling Write (instead of WriteLine) with ShowPercent enabled does not leave the cursor position in a proper position: the next thing that is written is written in the percentage label area.
• The redraw logic does not take value decrements (negative increments, or just assigning a lower value to Value) into account.

Other notes

• Write and WriteLine must be called first, to set the progress bar location, and then Increment must be called to redraw it. This means duplicate drawing code, and it's easy to use incorrectly (calling Increment before Write, or not calling Write at all). Why not combine all these into a single Draw method?
• Why is the redraw method called Increment? Why does it combine drawing with adjusting the value? The value can already be adjusted via the Value property, and there are various other properties that affect rendering too.
• Width is actually a percentage - setting it to 50 makes the progress bar half as wide as the console window. This isn't clear from the name, so I would add a little explanation (comment) there. You may also want to explain that the bar doesn't resize when the console window is resized.
• It's good that you're taking thread-safety seriously, but I'm not sure how much protection you're hoping to gain with that lock, since other code (outside your control) can still freely access Console. Creating a thread-safe wrapper around Console and documenting that all console interaction should be done via that wrapper should be more robust. It could offer atomic write methods that take a position and color, so calling code doesn't need to do any locking itself.
• There's no need for explicit backing fields for properties that don't do anything special in their getter and setter. If you do need an explicit backing field (such as _width) then personally I'd put it close to its associated property, not grouped together with other private fields.
• $" {progress.ToString("0.00")}%" can be written more succinctly as $" {progress:0.00}%".
• There's no need to set console colors inside those drawing loops. In fact, there's no need for loops at all - preparing a string (or char array) beforehand allows you to reduce the number of Console.Write calls, which should be faster.
• Abbreviations like csrtop may be slightly faster to write, but they don't improve the readability of the code. Just write cursorTop.
• You use a leading underscore for private fields, but sometimes also for local variables. Being inconsistent generally makes code harder to read (that is, it takes more time and work to understand).
• Instead of implicitly using 0 as minimum value, you may want to add a MinValue property.
• Do you have a specific reason for restricting Value to integers?

• Pieter Witvoet: @AdrianoRepetti: each individual console operation is safe, but the progress bar redrawing as a whole is not. What if another thread writes some text when the progress bar has been redrawn, but the console position hasn't been restored yet? Or in-between two Console.Write calls?
• Adriano Repetti: that's true, you're right!