# Color-coded console output

I wrote this Console.Write function to use in my applications where I need to easily color-code output.

Example syntax would be:

Console.WriteLine("<f=red>this is in red");
Console.WriteLine("<b=gray><f=white>This is white on gray. <f=green>This is green on gray. <b=darkmagenta>This is green on dark magenta. <b=d>This is green on default.");


Which results in this...

I'm looking for a general review of the code.

//github.com/BenVlodgi/CommandHelper
public static class Console
{
private static Regex _writeRegex = new Regex("<[fb]=\\w+>");

public static void WriteLine(string value, int? cursorPosition = null, bool clearRestOfLine = false)
{
Write(value + Environment.NewLine, cursorPosition, clearRestOfLine);
}

public static void Write(string value, int? cursorPosition = null, bool clearRestOfLine = false)
{
if (cursorPosition.HasValue)
System.Console.CursorLeft = cursorPosition.Value;

ConsoleColor defaultForegroundColor = System.Console.ForegroundColor;
ConsoleColor defaultBackgroundColor = System.Console.BackgroundColor;

var segments = _writeRegex.Split(value);
var colors = _writeRegex.Matches(value);

for (int i = 0; i < segments.Length; i++)
{
if (i > 0)
{
ConsoleColor consoleColor;
// Now that we have the color tag, split it int two parts,
// the target(foreground/background) and the color.
var splits = colors[i - 1].Value
.Trim(new char[] { '<', '>' })
.Split('=')
.Select(str => str.ToLower().Trim())
.ToArray();

// if the color is set to d (default), then depending on our target,
// set the color to be the default for that target.
if (splits[1] == "d")
if (splits[0][0] == 'b')
consoleColor = defaultBackgroundColor;
else
consoleColor = defaultForegroundColor;
else
// Grab the console color that matches the name passed.
// If none match, then return default (black).
consoleColor = Enum.GetValues(typeof(ConsoleColor))
.Cast<ConsoleColor>()
.FirstOrDefault(en => en.ToString().ToLower() == splits[1]);

// Set the now chosen color to the specified target.
if (splits[0][0] == 'b')
System.Console.BackgroundColor = consoleColor;
else
System.Console.ForegroundColor = consoleColor;
}

// Only bother writing out, if we have something to write.
if (segments[i].Length > 0)
System.Console.Write(segments[i]);
}

System.Console.ForegroundColor = defaultForegroundColor;
System.Console.BackgroundColor = defaultBackgroundColor;

if (clearRestOfLine)
ClearRestOfLine();
}

public static void ClearRestOfLine()
{
int winTop = System.Console.WindowTop;
int left = System.Console.CursorLeft;
System.Console.Write(new string(' ', System.Console.WindowWidth - left));
System.Console.CursorLeft = left;
System.Console.CursorTop--;
System.Console.WindowTop = winTop;
}
}

• I think you might be interested in this project github.com/DITLeonel/GetBootstrap Apr 15, 2015 at 18:56
• and basically all of this guy's questions codereview.stackexchange.com/users/29034/leonel?tab=questions Apr 15, 2015 at 18:57
• Didn't your mother ever tell you to use Curly Braces?
– Malachi
Apr 15, 2015 at 19:08
• @RubberDuck Boot strap is neat, but I've written my own methods like those before, and they still have the problem being separate calls for each color desired. I wrote this so that I could inline the changing of colors. Apr 15, 2015 at 19:32
• @YvesSchelpe yes, but I haven't been maintaining it very well. It is here on GitHub. Jan 25, 2016 at 20:02

public static class Console


Where's your using directives? They're needed for the code to compile; you should post them.

It's a little bit unusual to give your class the same name as one in System but I see why you did it -- so you can just switch to using your new module with a using.

{
private static Regex _writeRegex = new Regex("<[fb]=\\w+>");


Don't forget to make it readonly. The pattern for that Regex should be specified as a verbatim string literal so you don't have to double the backslashes (only you can prevent Leaning Toothpick Syndrome) -- @"<[FB]\w+>". You don't save length but it's easier to understand.

I had initially thought that it might make sense to construct this Regex using RegexOptions.Compiled, but it turns out that has no benefit since it's only used in a static method, so it will automatically be cached. But if you ever use nonstatic methods, it's worth trying it and see if it gives a speed increase.

    public static void WriteLine(string value, int? cursorPosition = null, bool clearRestOfLine = false)
{
Write(value + Environment.NewLine, cursorPosition, clearRestOfLine);
}


It's inefficient to use + here to append onto the string. Since .NET System.String is immutable, it has to build a whole new string and copy. It would be better to just call Write with the parameters as you got them, followed by System.Console.Write(Environment.NewLine);.

    public static void Write(string value, int? cursorPosition = null, bool clearRestOfLine = false)
{
if (cursorPosition.HasValue)
System.Console.CursorLeft = cursorPosition.Value;

ConsoleColor defaultForegroundColor = System.Console.ForegroundColor;
ConsoleColor defaultBackgroundColor = System.Console.BackgroundColor;

var segments = _writeRegex.Split(value);
var colors = _writeRegex.Matches(value);


This seems a bit inefficient since you're doing the same matching twice. I strongly suspect that it would be better to skip the .Split call and use a foreach loop on the Matches.

        for (int i = 0; i < segments.Length; i++)
{
if (i > 0)


As another reviewer suggested, this is suboptimal. If i==0 is a special case, move it outside the loop so you don't need to test i each time through the loop. (The compiler will probably do this for you, so you won't gain any speed -- it's just for code legibility)

            {
ConsoleColor consoleColor;
// Now that we have the color tag, split it int two parts,
// the target(foreground/background) and the color.
var splits = colors[i - 1].Value
.Trim(new char[] { '<', '>' })
.Split('=')
.Select(str => str.ToLower().Trim())
.ToArray();


This is clever, but it really would be easier and clearer to just use capturing groups in your Regex so you can access the two parts directly from the Match.

                // if the color is set to d (default), then depending on our target,
// set the color to be the default for that target.
if (splits[1] == "d")
if (splits[0][0] == 'b')


This code would be a lot clearer if you used meaningful variable names instead of splits[1] and splits[0].

                        consoleColor = defaultBackgroundColor;
else
consoleColor = defaultForegroundColor;


You like terseness -- how about the ternary operator here?

                else
// Grab the console color that matches the name passed.
// If none match, then return default (black).
consoleColor = Enum.GetValues(typeof(ConsoleColor))
.Cast<ConsoleColor>()
.FirstOrDefault(en => en.ToString().ToLower() == splits[1]);


Again, very clever, but I think it would be more efficient to just build a Dictionary<string,ConsoleColor> one time at the start, rather than searching the whole list each time. LINQ is extremely useful; that doesn't mean it's best used everywhere.

                // Set the now chosen color to the specified target.
if (splits[0][0] == 'b')
System.Console.BackgroundColor = consoleColor;
else
System.Console.ForegroundColor = consoleColor;
}

// Only bother writing out, if we have something to write.
if (segments[i].Length > 0)
System.Console.Write(segments[i]);
}


Like the other reviewer said, this if should be at the start so you don't bother unpacking the string at all.

        System.Console.ForegroundColor = defaultForegroundColor;
System.Console.BackgroundColor = defaultBackgroundColor;


How about System.Console.ResetColor()? It would make these two variables unnecessary.

        if (clearRestOfLine)
ClearRestOfLine();
}

public static void ClearRestOfLine()
{
int winTop = System.Console.WindowTop;
int left = System.Console.CursorLeft;
System.Console.Write(new string(' ', System.Console.WindowWidth - left));
System.Console.CursorLeft = left;
System.Console.CursorTop--;
System.Console.WindowTop = winTop;
}
}


Does this do what you want after a WriteLine? It looks to me like it clears the next line not the current one.

• Thanks @Snowbody, you brought up some very good points. WriteLine does not work as I intended, though it does, if I take your recommondation to use a separate System.Console.WriteLine(); after 'Writeing out the text. Apr 16, 2015 at 15:02
• I keep the currently set console colors as the defaults, because they may be different from the actual console defaults. On top of that, I need to be able to just set one at a time and not both. Apr 16, 2015 at 15:55

you should definitely be using Curly Braces here

            // if the color is set to d (default), then depending on our target,
// set the color to be the default for that target.
if (splits[1] == "d")
if (splits[0][0] == 'b')
consoleColor = defaultBackgroundColor;
else
consoleColor = defaultForegroundColor;
else
// Grab the console color that matches the name passed.
// If none match, then return default (black).
consoleColor = Enum.GetValues(typeof(ConsoleColor))
.Cast<ConsoleColor>()
.FirstOrDefault(en => en.ToString().ToLower() == splits[1]);
`

You have comments and multilined, single line code inside of an else statement. This calls for Curly Braces

and the if/else block inside of another if block, and no Curly Braces?! this is begging for Curly Braces.

• I abhor curly braces. But I expected nothing less. Apr 15, 2015 at 19:34
• Abhor them all you want, but there's no getting rid of them when you have more than one line. Which is a lot of code. Apr 16, 2015 at 4:23
• @MillieSmith and until I have more than one line of code, I will not add them Apr 16, 2015 at 13:48
• an if and else statement counts as more than one line of code inside of another if statement, even if it compiles the way you anticipate, it is still a bad habit to remove the curly braces from such a block of code. but hey maybe you would like VB or Python much better than C# or Java...
– Malachi
Apr 16, 2015 at 13:58