Display Loading Text with Spinner in Console

Question

Here is my approach to creating a loading text spinner in the console. I did what I thought was best in designing the class, which means there is probably a lot of room for feedback.

The class is below, along with it's usage inside a Utils class. You can also find it here on github if that's easier.

Concerns

I have posted a question on StackOverflow regarding thread-safety/other issues regarding the Task API, since my first usage of it is surely wrong.

Post-writing the code, I also considered returning strings from the Task instead of forcing it to be Console output, but did not implement this functionality.

I am primarily interested in your feedback concerning the code style and general design. Pointers on the asynchrony are welcome, but not mandatory for a good answer.

Layout

ConsoleLoadingText: class to encapsulate functionality.

• Constants
  • Defaults for ProductName, LoadingText, and MillisecondsDelay
  • Array of spinner pieces
• Constructors
  • Chained together constructors allow you to provide some or all of the data, relying on defaults to fill in the gaps. They all delegate to one constructor which does the work.
• Methods
  • Display returns a Task which, when run, does the display work
  • Stop stops the display

Utils: Utility class

• Methods
  • CreateLoading creates an instance of the ConsoleLoadingText class with the parameters passed in; uses optional parameters for maximum flexibility

Code

ConsoleLoadingText.cs

namespace Knoble.Utils
{
 /// <summary>
 /// A class that represents a possibily infinitely looping load screen.
 /// It displays a product name, loading text, and spinner that spins for a given delay.
 /// </summary>
 public class ConsoleLoadingText
 {
 public const string DefaultProductName = "";
 public const string DefaultLoadingText = "Loading...";
 public const int DefaultMillisecondsDelay = 250;
 static string[] spinner = { "|", "/", "-", "\\" };
 readonly string productName, loadingText;
 readonly int millisecondsDelay;
 int i;
 bool @continue = true;
 /// <summary>
 /// Initializes a new instance of the <see cref="T:Knoble.Utils.Loading"/> class.
 /// Defaults to displaying "Loading... x" where the spinner (x) spins every quarter second.
 /// </summary>
 public ConsoleLoadingText () : this (DefaultProductName)
 {
 }
 /// <summary>
 /// Initializes a new instance of the <see cref="T:Knoble.Utils.Loading"/> class.
 /// Defaults to displaying "{productName} Loading... x" where the spinner (x) spins every quarter second.
 /// </summary>
 /// <param name="productName">Product name.</param>
 public ConsoleLoadingText (string productName) : this (productName, DefaultLoadingText)
 {
 }
 /// <summary>
 /// Initializes a new instance of the <see cref="T:Knoble.Utils.Loading"/> class.
 /// Defaults to displaying "{productName} {loadingText} x" where the spinner (x) spins every quarter second.
 /// </summary>
 /// <param name="productName">Product name.</param>
 /// <param name="loadingText">Loading text.</param>
 public ConsoleLoadingText (string productName, string loadingText) : this (productName, loadingText, DefaultMillisecondsDelay)
 {
 }
 /// <summary>
 /// Initializes a new instance of the <see cref="T:Knoble.Utils.Loading"/> class.
 /// Displays "{productName} {loadingText} x" where the spinner (x) spins every {millisecondsDelay} milliseconds.
 /// </summary>
 /// <param name="productName">Product name.</param>
 /// <param name="loadingText">Loading text.</param>
 /// <param name="millisecondsDelay">Milliseconds delay.</param>
 public ConsoleLoadingText (string productName, string loadingText, int millisecondsDelay)
 {
 if (productName == null)
 throw new ArgumentException (nameof (productName));
 if (loadingText == null)
 throw new ArgumentException (nameof (loadingText));
 if (millisecondsDelay < 0)
 throw new ArgumentException (nameof (millisecondsDelay));
 this.productName = productName;
 this.loadingText = loadingText;
 this.millisecondsDelay = millisecondsDelay;
 }
 /// <summary>
 /// Returns a task that, when running, continously prints the loading text.
 /// </summary>
 public Task Display ()
 {
 return Task.Run (() =>
 {
 @continue = true;
 while (@continue)
 {
 Console.Write ($"\r{productName} {loadingText} {spinner[i]}");
 i = (i + 1) % spinner.Length;
 Thread.Sleep (millisecondsDelay);
 }
 });
 }
 /// <summary>
 /// Stop this instance from displaying.
 /// </summary>
 public void Stop ()
 {
 @continue = false;
 }
 }
}

Utils.cs

namespace Knoble.Utils
{
 public static class Utils
 {
 /// <summary>
 /// Creates a <see cref="T:Knoble.Utils.Loading"/> object for displaying loading text to the console.
 /// </summary>
 /// <returns>The <see cref="T:Knoble.Utils.Loading"/> object.</returns>
 /// <param name="productName">Product name.</param>
 /// <param name="loadingText">Loading text.</param>
 /// <param name="millisecondsDelay">Milliseconds delay.</param>
 public static ConsoleLoadingText CreateLoading (string productName = ConsoleLoadingText.DefaultProductName, string loadingText = ConsoleLoadingText.DefaultLoadingText, int millisecondsDelay = ConsoleLoadingText.DefaultMillisecondsDelay)
 {
 return new ConsoleLoadingText (productName, loadingText, millisecondsDelay);
 }
 }
}

• D. Ben Knoble: If anyone knows more relevant tags, please suggest or add them
• t3chb0t: This doesn't spin, it just prints the string in a loop.
• D. Ben Knoble: The "\r" causes it to overwrite the previous string each time. @t3chb0t
• t3chb0t: OK, my bad, didn't see this one ;-)

Answer 1

Your constructors are really weird. You require all of the variables, but you still offer separate constructors which accept just a few parameters and you also have a helper class which utilises only the full ctor. It makes more sense to have a single constructor with no additional overloads.

If you want to build objects parameter per parameter you should look at the builder pattern reference.

But if that wasn't your intention just leave this constructor :

public ConsoleLoadingText(string productName, string loadingText, int millisecondsDelay)
{
 if (productName == null)
 throw new ArgumentException(nameof(productName));
 if (loadingText == null)
 throw new ArgumentException(nameof(loadingText));
 if (millisecondsDelay < 0)
 throw new ArgumentException(nameof(millisecondsDelay));
 this.productName = productName;
 this.loadingText = loadingText;
 this.millisecondsDelay = millisecondsDelay;
}

Code style

Naming

@continue that's a weird name looking at the @ a underscore would be more appropriate

But why is only this variable special ? I don't see any reason why only this variable would have @ at the beginning. You should be consistent in your variable naming, you either make all of them start with underscore or none of them. This makes reading long classes easier, because once you get used to specific convention, you can tell the modifiers of that variable just by looking at the name.

Modifiers

I prefer having all of my members with explicit access modifiers.

spinner can be a readonly variable and you can use a verbatim string there for the last argument :

private static readonly string[] spinner = { "|", "/", "-", @"\" };

• D. Ben Knoble: The @ prefix was because continue is a keyword, but the underscore is a good replacement thanks.
• t3chb0t: @DavidBenKnoble continue is to general, I suggest keepSpinning or continueSpinning.
• D. Ben Knoble: denis, if no other answers are posted within the next day, i'll accept yours
• Denis: Thank you @DavidBenKnoble, if there is anything unclear feel free to ask me.