How to factor static call out of a class

Question

Let's say I have a static method called Logger.log(), which calls another static method, CurrentUser.getName(), to get some additional information to log:

public static void log(text) {
  String[] itemsToLog = { text, todaysDate, ipAddress, CurrentUser.getName() };

Now obviously this isn't an ideal situation, especially with the static data in the CurrentUser class. But I want to start improving it by reducing the Logger's dependencies. I'd prefer that the Logger not have any knowledge of higher-level concepts like users. It just needs a list of things to log, and doesn't care what they are.

So I want to somehow factor out the CurrentUser class. But Logger is static, so I can't just pass the information into its constructor.

What would be a good pattern for factoring out things like this?

Answer 1

You have two options:

1. Always pass the information to Logger
2. Have Logger maintain it statically within Logger (or call another method)

If you don't want Logger to maintain it statically, and you don't want to include the additional information (or calls) in the call each time, then you could create another class that calls Logger and passes all that static information, then change Logger to have no static data (or at least not call CurrentUser). Then the class that calls logger could accept CurrentUser in its constructor.

You might use that as a stepping stone for a future refactoring.

If your language supports extension methods or class helpers, then you could change Logger to accept CurrentUser as a parameter, and then add an extension method that only accepts the log text, and then passes the CurrentUser automatically. This would allow you to make the change without changing all the calls, although it would require that the extension method be static . . . so you don't gain much ground.

Answer 2

You don't have a lot of choices here. If you don't want the logger to rely on the CurrentUser, you can rely on the end user (logging class) to insert the user in the logging text, or create a sub-class of logger which does know about CurrentUser, which may defeat your purpose.

Answer 3

I'd prefer that the Logger not have any knowledge of higher-level concepts like users.

It sounds like the direction you may want to go is to separate log message composition and formatting logic from the logging mechanics. For example (pardon my C# idioms):

public class WebRequestLogEntry {

    // In some frameworks, you may get username, address, etc. from an
    // HttpContext or similar object, simplifying this constructor

    public WebRequestLogEntry(string message, string userName, IpAddress address) {
        // Sets member variables
    }

    public string Text {
        get {
            // Concatenate and format member data
        }
    }
}

From there, just call your logger like this:

Logger.log(new WebRequestLogEntry("Hi", CurrentUser.getName(), ipAddress).Text);

Answer 4

It seems to me that your Logger already maintains some state (e.g., date, address, user, etc.).

Wouldn't it make sense to make log() a nonstatic call on a specific logger, and have everything relevant (including user) initialized when the logger is first created? You could have a loggers manager which you would use to initialize and later obtain specific loggers, or just make your logger a singleton (if it is such). The logic of obtaining the user would then be in the loggers manager or in the factory/getInstance() for the logger, rather than in the Logger instance itself.

Answer 5

One option could be to create a new interface ContextProvider and get your contextual string from there

public interface ContextProvider{
    public List<String> getContextToLog();
}
...
public class DefaultLoggingContext implements ContextProvider{
    public List<String> getContextToLog(){
        ...
        list.Add(CurrentUser.getName());
        ...
        return list;
    }
}
...
public class Logger{
    private static ContextProvider contextProvider;

    public static initiliseLogger(ContextProvider defaultProvider){
        contextProvider = defaultProvider;
    }

    public static log(String text){
        log(text, contextProvider);
    }

    public static log(String text, contextProvider){
        List<String> toLog = contextProvider.getContextToLog();
        toLog.add(text);
}
...
public class ...{
    private ContextProvider loggingContext; // set by a constructor, factory method or a IOC container

    private onApplicationStart(){
        Logger.initiliseLogger(loggingContext)
    }
}

You could take this a step further and use a Formatter instead of a contextProvider the formatter would be responsible for taking the input string "text" and formatting it compleatly including adding any session, date, time, request etc information. You can look at log4* for full implamentations of this.

On a side note I would suggest that moving the log method to be a instance method not a static method would a very good move, you can either support both for a while with the static marked as depricated or you can get out your find and replace stick. One feature of log4* that I really like is the ability to change logging sensitivity based on class or package.