1
\$\begingroup\$

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 {
        trace.Fatalln(err)
        return
    }
    trace.Println(errorLog)
    // save the log out put to file
    trace.SetOutput(f)
    trace.Fatalln(errorLog, err)
}

To use this package I must call the InitLogger function first

logger.InitLogger("Prefix Logger","./filpath/","filename.txt")

And then after that I can choose to print the error using either PrintLog or FatalLog.

The PrintLog function is used when we wanted to print the log without having crashing or closing the app os.Exit. The FatalLog the same as PrintLog but will crash the app or call os.Exit.

Any idea what can be improved here? source code

\$\endgroup\$
1
\$\begingroup\$
  • 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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.