trjhtr

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.

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;
 
 
 
 

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?