Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link
 private string GetLogDateTime()
 {
 return "[" + DateTime.Now.ToLongTimeString() + "] ";
 }

The user should be able to define whether he wans to log local or utc time. If there is any doubt then I would log the utc.

Another issue here is that you don't log the timestamp as invariant. Its format will change between EN and DE. If you will need to import/compare logs from two different worlds it will be really hard.


 public void Log(string line, LogType logType)
 {
 ConsoleColor existingColor = Console.ForegroundColor;
 Console.WriteLine(GetLogDateTime() + line);
 Console.ForegroundColor = existingColor;
 if (logSettings.TypesToSave.Contains(logType))
 WriteContentToFile(logSettings.LogFile, line);
 }

You need to split it into configurable modules so the user can decide whether he wants to log to the console and/or to the file. Currently he has no choice and if he uses your library in a console project it will destroy his console output by writing some logs there.

There are other issues but @Heslacher @Heslacher has already mentioned them.

 private string GetLogDateTime()
 {
 return "[" + DateTime.Now.ToLongTimeString() + "] ";
 }

The user should be able to define whether he wans to log local or utc time. If there is any doubt then I would log the utc.

Another issue here is that you don't log the timestamp as invariant. Its format will change between EN and DE. If you will need to import/compare logs from two different worlds it will be really hard.


 public void Log(string line, LogType logType)
 {
 ConsoleColor existingColor = Console.ForegroundColor;
 Console.WriteLine(GetLogDateTime() + line);
 Console.ForegroundColor = existingColor;
 if (logSettings.TypesToSave.Contains(logType))
 WriteContentToFile(logSettings.LogFile, line);
 }

You need to split it into configurable modules so the user can decide whether he wants to log to the console and/or to the file. Currently he has no choice and if he uses your library in a console project it will destroy his console output by writing some logs there.

There are other issues but @Heslacher has already mentioned them.

 private string GetLogDateTime()
 {
 return "[" + DateTime.Now.ToLongTimeString() + "] ";
 }

The user should be able to define whether he wans to log local or utc time. If there is any doubt then I would log the utc.

Another issue here is that you don't log the timestamp as invariant. Its format will change between EN and DE. If you will need to import/compare logs from two different worlds it will be really hard.


 public void Log(string line, LogType logType)
 {
 ConsoleColor existingColor = Console.ForegroundColor;
 Console.WriteLine(GetLogDateTime() + line);
 Console.ForegroundColor = existingColor;
 if (logSettings.TypesToSave.Contains(logType))
 WriteContentToFile(logSettings.LogFile, line);
 }

You need to split it into configurable modules so the user can decide whether he wants to log to the console and/or to the file. Currently he has no choice and if he uses your library in a console project it will destroy his console output by writing some logs there.

There are other issues but @Heslacher has already mentioned them.

Source Link
t3chb0t
  • 44.6k
  • 9
  • 84
  • 190
 private string GetLogDateTime()
 {
 return "[" + DateTime.Now.ToLongTimeString() + "] ";
 }

The user should be able to define whether he wans to log local or utc time. If there is any doubt then I would log the utc.

Another issue here is that you don't log the timestamp as invariant. Its format will change between EN and DE. If you will need to import/compare logs from two different worlds it will be really hard.


 public void Log(string line, LogType logType)
 {
 ConsoleColor existingColor = Console.ForegroundColor;
 Console.WriteLine(GetLogDateTime() + line);
 Console.ForegroundColor = existingColor;
 if (logSettings.TypesToSave.Contains(logType))
 WriteContentToFile(logSettings.LogFile, line);
 }

You need to split it into configurable modules so the user can decide whether he wants to log to the console and/or to the file. Currently he has no choice and if he uses your library in a console project it will destroy his console output by writing some logs there.

There are other issues but @Heslacher has already mentioned them.

lang-cs

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