0

Consider I am going to write a simple file based logger AppLogger to be used in my apps, ideally it should be a singleton so I can call it via

public class AppLogger {
    public static String file = "..";

    public void logToFile() {
        // Write to file
    }

    public static log(String s) {
        AppLogger.getInstance().logToFile(s);
    }
}

And to use it

AppLogger::log("This is a log statement");

The problem is, what is the best time I should provide the value of file since it is a just a singleton?

Or how to refactor the above code (or skip using singleton) so I can customize the log file path? (Assume I don't need to write to multiple at the same time)

p.s. I know I can use library e.g. log4j, but consider it is just a design question, how to refactor the code above?

  • 2
    the question is tagged Java, but AppLogger::log doesn't look like a valid Java syntax, why so? – gnat Aug 19 '14 at 7:13
  • 1
    Initialization of the file link needs to be one of the first things your app does - otherwise you may end up calling log before there's an output file (a good library probably has a built-in buffer or at least a default destination file). If you're building a GUI app, you can probably do it before the launch of the UI via the Swing threads. Most servers come with the logging built-in, so it's not something you have to worry about. – Clockwork-Muse Aug 19 '14 at 7:45
  • why not just use an existing logging framework like log4j? – jk. Aug 19 '14 at 16:25
2

I think you could just go ahead and provide a setter Method for the file path in the AppLogger. If you're using multiple threads in your application, make sure to keep the methods synchronized, as a Singleton is a shared object that can be accessed by all threads:

public static void synchronized setFile(String filePath) {...}

public static void synchronized log(String s) {...}

Some other points:

  • AppLogger::log is not correct Java syntax
  • You have two methods for logging, and both are public. Make logToFile private and add a string argument
  • The actual singleton reference is missing in your class
  • The signature of log(String) is not correct. You have to add a return type (like I did)
  • synchronized, while it will make things thread-safe, is likely to slow logging significantly (waiting for other threads). Depending on the design, you might even get into some form of deadlock, which would be extremely painful. Besides which, it wouldn't actually protect you from the biggest issue - calling log before setFile. – Clockwork-Muse Aug 19 '14 at 7:39
  • Exactly right. Being a singleton has nothing to do with being immutable; it just means that there is exactly one instance, not exactly the same configuration forever. If you need to be able to change the log file path, go ahead and make it settable, and if you need to guard against concurrent access, the normal threading rules apply. – Kilian Foth Aug 19 '14 at 8:12
  • Yeah but just because you can safely modify a Singleton doesn't mean it's a good idea. Mutable singletons are hard to test and can lead to hard-to-reproduce runtime bugs. – Matthew May 29 at 15:19
1

Allowing the file to change anytime raises a lot of red flags, such as major synchronization hassles.

But, even worse, what happens if Thread A is merrily logging 4 lines of

64.242.88.10 - - [07/Mar/2004:16:05:49 -0800] INFO:blah
INFO: Opening some connection to foo.bar.com
INFO: Connection established with 123.45.67.8
ERROR: Connection refused by socket 5432, bad credentials

and in between line 1 and 2 (or maybe in the middle of them depending on the granularity) Thread B calls setFile(someCompletelyDifferentFile)? Even if you synchronize per-line writes, this will be a mess.

Your logger should be configured early, preferably at application startup, before anybody starts using it, and then not change files in midstream.

0

It depends whether the file name is dynamic and can change any time or whether the fil e name is set at initialization time.

If you want a one-time initialization, you would set the filename in your app's initialization code. Just ignore any call to logToFIle before the filename has been set.

Or you could set the file name in the constructor or in getInstance(). The singleton should then be able to initialize itself by retrieving the file name from a configuration file.

On the other side, if the file name can change while the app runs, you have to handle it like any other mutable state. Access the singleton by getInstance() and call the setFilename() method.

In any case, if multiple thread access is a possibility, all operations must be synchronized. If you are logging, the writing of the log file would probably take much more time than the synchronization.

0

There are two conditions for your logger (and for all singletons): 1. The singleton should be callable at any time. 2. The singleton should be callable at any time, from any thread.

(1) means that you cannot expect to set the filename before the singleton gets called the first time. You can set it of course while the singleton is being created, from the code creating the singleton. In other words, not from the outside. (2) means everything has to be done in a thread-safe way, using mutexes, synchronisation or whatever.

What you could do in this particular case: As long as the filename is not set, buffer all the data that should be logged. When the filename is set for the first time, open a logging file, and dump all the buffers to the logging file. If the filename is set later again to a different name, close the logging file and open a new one, and all following logging statements go to the new file.

You need to be aware obviously that if thread A changes the filename while thread B is logging various things, some of the things thread B logs may end up in one file, and some in another.

And maybe if you want to log to different files, you might not use a singleton. Or look at it not as a "singleton" but as a "special purpose object", so you might have two or three of them. One for logging database access, one for logging UI, and another one for logging authentication. Three objects, and you pick which one to use depending on circumstances.

-1

The right time to set the file name is any time before the first call to the Log method. Just throw an exception from there if the object has not been initialized yet.

  • The problem with that is that as a Singleton you can't guarantee when the first call to log happens and it's excessive to have to try/catch every call to log. – Matthew May 29 at 16:43
  • I am not talking about try-catch, just throw. It is not supposed to happen, the exception is there to alert the programmer who uses the log object he should provide a file name first. The only overhead is a check for null. – Martin Maat May 29 at 16:57
  • But because it's a singleton, it's possible that the first time log is invoked is different for different executions. So the programer would either constantly check if the file is null (and if so, how, in the depths of code, do you know what to set it to) or set the filename at the beginning of the program. In which case, why make it mutable? Alternately, if the OP wants to log to different files, don't make the class a singleton. – Matthew May 29 at 18:50
-1

You can change the properties of the singleton object this way.

singleton.js

export var SingletonTester = (function () {

    // our instance holder  
    var instance;

    // an emulation of static variables and methods
    var _static  = {   

      name:  "SingletonTester",

      getInstance:  function() { 

        if( instance  ===  undefined )  {     
          instance = this;   
        }    

        return  instance;  

      },

      getname: function(){
          return this.name;
      },

      setname: function(name){
            this.name = name;
      }
    };  

    return  _static;

  })();

In main.js

    var iA = SingletonTester.getInstance();
    var iB = SingletonTester.getInstance();

    var nameA = iA.getname();
    var nameB = iB.getname();

    console.log(nameA, "   ", nameB); 
    //Output
    //SingletonTester  SingletonTester

    var iC = SingletonTester.getInstance();
    iC.setname("SingletonTester is Changed");

    console.log(iA.getname(), "   ", iB.getname(), "  ",iC.getname());
    //Output
    //SingletonTester is Changed  SingletonTester is Changed  SingletonTester is Changed

Here "instance" is a private property of "SingletonTester" and "name", "setname", "getname", "getInstance" are public properties.

with the getInstance method, it will first check if the SingletonTester's object is created already, if yes it will just return the reference to the object, or it will create new and return it.

That way, iA and iB are pointing to the same object. If you try this,

console.log(iA == iB);  //it will return true

changing the instance variable through one object will change/updates the second instance too.

  • Perhaps you could add some explanation to the code? This way you make it easier to understand what it is you're trying to explain. Just a code sample won't have a high educational value. – Onno May 31 at 12:29

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.