Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###private static void Render()

private static void Render()

###private static void Render()

private static void Render()

Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

The code in question looks mostly good to me, but it could be enhanced a little bit.

###private static void Render()

The call to Console.ResetColor(); doesn't belong here because it isn't necessary to reset the colors each time.

Remarks from link above:

The foreground and background colors are restored to the colors that existed when the current process began.

So it is sufficient to call that method at the end of the Render(IEnumerable<XNode> xNodes) method.

The passed in lastForegroundColor and lastBackgroundColor are only needed if one of the childnodes is a XElement so if we extract the rendering of a XElement to a separate method, the former Render() method would look after renaming to RenderInternal() like so

private static void RenderInternal(IEnumerable<XNode> xNodes)
{
 foreach (var xChildNode in xNodes)
 {
 var xElement = xChildNode as XElement;
 if (xElement != null)
 {
 RenderInternal(xElement);
 }
 else
 {
 Console.Write(((XText)xChildNode).Value);
 }
 }
} 

The extracted RenderInternal(XElement) method could look like so, if we just use the current implemented methods

private static void RenderInternal(XElement xElement)
{
 ConsoleColor lastForegroundColor = Console.ForegroundColor;
 ConsoleColor lastBackgroundColor = Console.BackgroundColor;
 SetForegroundColor(xElement);
 SetBackgroundColor(xElement);
 RenderInternal(xElement.Nodes());
 RestoreForegroundColor(lastForegroundColor);
 RestoreBackgroundColor(lastBackgroundColor);
} 

which removes the strangeness of a SetXxx method to return something. But hey, I just don't like how this is looking. So let us introduce a struct ConsoleColors to hold both the Foreground- and the Backgroundcolor like so

public struct ConsoleColors
{
 public ConsoleColor BackgroundColor { get;}
 public ConsoleColor ForegroundColor { get;}
 public static ConsoleColors Current
 {
 get
 {
 return new ConsoleColors(Console.ForegroundColor, Console.BackgroundColor);
 }
 }
 
 public ConsoleColors(ConsoleColor foregroundColor, ConsoleColor backgroundColor)
 :this()
 {
 ForegroundColor = foregroundColor;
 BackgroundColor = backgroundColor;
 }
} 

and add a method which sets a ConsoleColors struct like so

private static void SetColors(ConsoleColors colors)
{
 Console.ForegroundColor = colors.ForegroundColor;
 Console.BackgroundColor = colors.BackgroundColor;
} 

and refactor the RenderInternal(XElement) method like so

private static void RenderInternal(XElement xElement)
{
 ConsoleColors savedColors = ConsoleColors.Current;
 SetForegroundColor(xElement);
 SetBackgroundColor(xElement);
 RenderInternal(xElement.Nodes());
 SetColors(savedColors);
}

which looks better, but still has the calls SetForegroundColor and SetBackgroundColor. So we can add an extension method which turns a XElement to a ConsoleColors so we can use the SetColors method.

public static ConsoleColors ToConsoleColors(this XElement xElement, string foregroundAttributeName = "fg", string backgroundAttributeName = "bg")
{
 if (xElement == null) { return ConsoleColors.Current; }
 var foregroundColor = Console.ForegroundColor;
 Enum.TryParse<ConsoleColor>(xElement.Attribute(foregroundAttributeName)?.Value, true, out foregroundColor);
 var backgroundColor = Console.BackgroundColor;
 Enum.TryParse<ConsoleColor>(xElement.Attribute(backgroundAttributeName)?.Value, true, out backgroundColor);
 return new ConsoleColors(foregroundColor, backgroundColor);
} 

which leads to

 internal class ConsoleColorizer
 {
 public static void Render(string xml)
 {
 Render(XElement.Parse(xml).Nodes());
 }
 public static void Render(IEnumerable<XNode> xNodes)
 {
 RenderInternal(xNodes);
 Console.ResetColor();
 }
 private static void RenderInternal(IEnumerable<XNode> xNodes)
 {
 foreach (var xChildNode in xNodes)
 {
 var xElement = xChildNode as XElement;
 if (xElement != null)
 {
 RenderInternal(xElement);
 }
 else
 {
 Console.Write(((XText)xChildNode).Value);
 }
 }
 }
 private static void RenderInternal(XElement xElement)
 {
 ConsoleColors savedColors = ConsoleColors.Current;
 SetColors(xElement.ToConsoleColors());
 RenderInternal(xElement.Nodes());
 SetColors(savedColors);
 }
 private static void SetColors(ConsoleColors colors)
 {
 Console.ForegroundColor = colors.ForegroundColor;
 Console.BackgroundColor = colors.BackgroundColor;
 }
 }
}
default

AltStyle によって変換されたページ (->オリジナル) /