HiveBrain v1.2.0
Get Started
← Back to all entries
debuggoMinor

Print log error and save it to a file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
errorfilelogsaveprintand

Problem

I have created a simple custom logger to print the error log and save it to a file with specific location.

```
package logger

import (
"log"
"os"
)

var trace *log.Logger
var filePathVar string
var fileNameVar string
var prefixVar string

// use this function first before calling others.
func InitLogger(prefix, filePath, fileName string) {
setPrefix(prefix)
createLogFile(prefix, filePath, fileName)
filePathVar = filePath
fileNameVar = fileName
prefixVar = prefix
}

func setPrefix(prefix string) {
trace = log.New(os.Stderr,
prefix,
log.Ldate|log.Ltime|log.Lshortfile)

}

func createLogFile(prefix, filePath, fileName string) (*os.File, error) {
setPrefix(prefix)
f, err := os.OpenFile(filePath+fileName, os.O_RDWR|os.O_CREATE|os.O_APPEND, os.ModePerm)
if err != nil {
// error file is not created
err = os.MkdirAll(filePath, os.ModePerm)
if err != nil {
trace.Println(err)
return nil, err
}
createLogFile(prefix, filePath, fileName)
}

return f, nil
}

// use this PrintLog use print the error on the terminal without exitting apps.
func PrintLog(errorLog string) {
//assign to global variable
if filePathVar == "" || fileNameVar == "" || prefixVar == "" {
trace.Fatalln("Error file path and Name empty prefix Cannot be Empty")
}

f, err := createLogFile(prefixVar, filePathVar, fileNameVar)
if err != nil {
trace.Fatalln(err)
return
}
trace.Println(errorLog)
// save the log out put to file
trace.SetOutput(f)
trace.Println(errorLog, err)
}

// use this Fatal log to exit the application whenever got error.
func FatalLog(errorLog string) {
//assign to global variable
if filePathVar == "" || fileNameVar == "" || prefixVar == "" {
trace.Fatalln("Error file path and Name empty")
}

f, err := createLogFile(prefixVar, filePathVar, fileNameVar)
if err != nil {

Solution


  • If some of your init values (prefix, filePath, fileName) are empty, you should fail on init, not once you try to log something. Returning errors as soon as possible is a good practice and makes debugging much easier.



  • setPrefix should be called setCustomLogger or something, or better, since it's a one-line function, just inline it.



  • You're calling setPrefix twice on init. createLogFile shouldn't call it.



  • Don't join paths using string concatenation. Use filepath.Join instead.



  • You should check that createLogFile succeeds on init, and fail fast otherwise.



  • InitLogger should simply be called Init, PrintLog should be Print, FatalLog should be Fatal: you're going to call them with logger.[…] from outside your package.



  • godoc comments should start with the name of the function they document: PrintLog prints a log message on the console, as well as in the log file… or something.



  • Why are you calling createLogFile many times? Just call it on init, save the file somewhere and use it each time. I think this also allows you to get rid of all these weird variables and replace them with only what you need (an open file).



  • Hoooo I understand why, it's because it needs to be reinitialized to os.Stderr. Damn, that's complex. Why not simply have two loggers (once initialized to os.Stderr and the other to your file, and simply call the corresponding function on each when your custom logger is called?



  • You're never closing your file. There is no way you can do that from your logging package (unless you open/close it every time you write a log line, but this is terribly inefficient), so maybe your Init function should take a os.File argument instead of a file path + file name. This way, the main function of your program (which will initialize the logger) can close it once the program is over. Of course, you should also close it when Fatal is called



In short: your Print method should be two lines long, your Fatal method should be three lines long, you shouldn't have more than 2 global variables, and all errors should be caught on Init.

Context

StackExchange Code Review Q#155600, answer score: 3

Revisions (0)

No revisions yet.