Logging System - Follow Up
ConsoleColor existingColor = Console.ForegroundColor;
PrintLine(line);
Console.ForegroundColor = existingColor;



This suggests something in PrintLine changes the Console.ForegroundColor... but nothing does, so why even bother with the foreground color at all?

Seems this would have the exact same effect:

PrintLine(line);


But let's pretend for a minute, that PrintLine takes an argument that does change the foreground color:

PrintLine(line, logType);


Whose job is it to cache the original ForegroundColor, and reset it? That's right, PrintLine should be responsible for that.

By abstracting the Console.WriteLine call behind a method call, you've introduced mixed abstraction levels in your Log method: you have low-level Console.ForegroundColor reads and writes, and a higher-level PrintLine call, in the same method: if Console.WriteLine is too low-level implementation details for Log, then so is Console.ForegroundColor.

Keep abstraction levels separated.



You're still omitting braces:


if (logSettings.TypesToSave.Contains(logType))
    WriteContentToFile(logSettings.LogFile, line);



Should be:

if (logSettings.TypesToSave.Contains(logType))
{
    WriteContentToFile(logSettings.LogFile, line);
}


XML comments are great, but completely useless on private members; private members don't show up in IntelliSense, so what's the use? At best, they're redundant - if you're looking at WriteContentToFile, you are looking at the source code. If the code needs such comments, you need to work on naming things properly - but WriteContentToFile, filePath and content are pretty darn crystal-clear.

Actually the XML comments are not so great:


/// &lt;summary&gt;
/// Prints a line to the console using custom settings.
/// &lt;/summary&gt;
/// &lt;param name="line"&gt;Line you want to print.&lt;/param&gt;
/// &lt;param name="logType"&gt;Type of line you want to print.&lt;/param&gt;



This is wrong. It's printing a line to the console and writing that line to a file, using hard-coded settings. This would be more accurate:

/// &lt;summary&gt;
/// Writes a new log entry.
/// &lt;/summary&gt;
/// &lt;param name="line"&gt;The text content of the log entry.&lt;/param&gt;
/// &lt;param name="logType"&gt;The type/level of log entry.&lt;/param&gt;


The notion of "printing" is an implementation detail that doesn't belong in the public interface or anywhere in the documentation: one day that method will be writing log entries to a database - would "printing" still apply?



I would say the other isses have already been pointed out in the previous iteration - and they're important issues too, and you haven't addressed them here.


Log timestamps are still culture-dependent.
Settings are still hard-coded in the constructor.
Using that logger in a console app will still wreck the app's output.