2

I'm writing a small C++ class, Block, to transform serialized data into a memory structure, and supply the structured data to callers through several accessor methods. I've tried to keep its scope specific and limited. The users of the class are very low-level - they, too, are very narrow in their focus and have as few external dependencies as possible.

This is how I've been taught to engineer things if at all possible. By minimizing dependencies and creeping featurism, it's easier to unit test, and easier to reuse.

The problem is that my class depends upon someone else's class, Metadata. It, too, does one very specific thing: it reads data that defines the characteristics of the data stream I'll be transforming from a database table and passes it to me. His class checks for mySQL errors, which should be rare, and logs any errors to a Log object.

This Log object appears in all of our company's applications. Instantiating it is a big deal - it wants Job numbers, it wants a lot of configuration information from the database that's normally put there in production by account managers using a GUI. You have to do a lot of work before your program can instantiate the Log. Yet this tiny, low-level class (Metadata) with one tiny task wants it to be passed in, by me. My object certainly has no business instantiating the Log, so I have to take it as a parameter from whoever calls me. And so forth, up the calling hierarchy.

I can understand why management wants a class to encapsulate and standardize message logging. But the need for it to be passed to, and through, just about every method is extremely ugly, and makes testing and reuse much more difficult.

This kind of problem must be fairly common. How can this be done without cluttering up the signature of every method you write? Is this a legitimate case for Globals? Is there some kind of Object Oriented approach? And, is this a God object?

Chap
  • 723
  • 1
  • 7
  • 18

4 Answers4

3

A God Object does everything for everyone. It's an unstructured program hiding in the guise of an OOP class, but one that is way too large by any measure.

In contrast, both your class and the Log are reasonable-size classes that do one thing and do it well. Your only problem is that the Log has to be available everywhere. If this is really the case, and it is clear that there will be only one Log object in the process, then making it global is the appropriate thing to do.

(There is a lot of hate for global variables (and for singleton objects) based on the horrible mess they make when used them for anything else, such as intermediate results, error codes, or even loop counters (shudder), but as always there's a spectrum of reasonable and unreasonable things you can do with them. Your case sounds like just about the most reasonable use for a global I can think of.)

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
3

Since the logger is some kind of infrastructure it is ok to not pass it around in every call.

In the java and c# world usually every class that need a logger has a static private logger that the class gets from a logger factory

using log4net;
namespace MyNamespace
{
   public class MyClass
   {
      private static readonly ILog log = log4net.LogManager.GetLogger("MyNamespace.MyClass");

      public void someFunction() {
         log.debug("someFunction was called");
      }

This approach assumes that the logger stays always the same instance so the performance overhead is minimal (only one factory call per class).

Alternativly a more dynamic logger that can be changed at runtime could be used through a public static method (singelton)

namespace MyNamespace
{
   public class MyClass
   {
      public void someFunction() {
         Global.getLogger().debug("someFunction was called");
      }

[update 2013-07-14]

answering chaps questions:

In the classlibrary log4net every class gets its own logger instance. The parameter in log4net.LogManager.GetLogger("MyNamespace.MyClass") is the name of the logger. through configuration you can tell the "MyNamespace.MyClass" logger - to be enabled/disabled - that every logging line should be prefixed with "MyNamespace.MyClass"

k3b
  • 7,488
  • 1
  • 18
  • 31
  • +1 Though I would still inject a logger, albeit not with every call, but more into every instance that needs one, +1 for the dynamic logger. That way you at least still get some testability and stubbability for all code that uses a logger. – Marjan Venema Jul 13 '13 at 10:53
  • I'm struggling to understand the concept of the Factory in general. In the first approach, why is typeof(MyClass) being passed? In the second approach, is a "Factory" involved at all? I'm liking this answer, esp. the 2nd since it appears I can make up a dummy Log class for testing purposes, but am having trouble understanding the notion of a Factory. – Chap Jul 13 '13 at 17:40
2

It may not be immediately obvious, but you've just described a large part of the motivation behind introducing exception handling into many languages. You have a low-level component that really shouldn't impose the policy the failure uses the details of some particular logging class. Rather, Metadata's job should simply be to either 1) do its job, or 2) signal failure. Knowledge of logging should be restricted to something much higher up the stack (both figuratively and, in this case, literally) that deals with things like application logs.

So, the preferable design is that Metadata simply throws an exception if it can't do its job. At some (much) higher level, code that will invoke something that uses Metadata sets up a handler for the exception that Metadata throws, and in the handler it logs the result. Using, for the moment, C++ syntax (but the basic idea applies equally to many other languages, we get something like this:

Log log(lots,of,inputs);

try {
     Metadata metadata(input);
     Block block(metadata);
     block.write(output);
}
catch (metadata_exception const &e) {
    log(e.whatever());
}

Result: Block and Metadata remain small, clean, and reusable. Logging happens at an appropriate level, instead of passing the Log object around to every low-level function.

Note one point though: this applies here largely because of one crucial point: you've made it clear that the logging happens only in case of an error. Throwing an exception in case of an error (i.e., you really can't continue what you were trying to do) is entirely appropriate and sensible. This pattern would not work if (for example) you were logging routine events. If (for example) you wanted to log successfully reading metadata from the database, this would not be appropriate at all.

Since you're logging failure, however, I will posit that (if available) throwing an exception is really the correct way to handle the situation -- definitely better than depending on a global Log object, having a log factory, etc.

Those are, I will posit, kludges to reduce the pain of a poorly chosen structure. They're roughly equivalent to heavy-duty bandages to minimize the blood loss after cutting off your arm. Yes, if you cut off your arm, good bandages might save your life -- but it's still a lot better to just not cut off your arm in the first place.

Jerry Coffin
  • 44,385
  • 5
  • 89
  • 162
  • 1
    I like your thinking here. Unfortunately, one of the arguments presented *for* logging directly from the low-level component was to support a logging level that might include DEBUG, in which case successful events might be logged. The need for this level might not be restricted just to the development phase. I have a hard time refuting this... so we're going with a global object. – Chap Jul 17 '13 at 17:34
  • @Chap: Yes -- if you have to log successes, that changes the situation entirely (and is where AOP starts to look like a much better idea). – Jerry Coffin Jul 17 '13 at 20:21
0

The mainstream way of dealing with a "global" logging object would be to use Dependency Injection.

The slightly less mainstream way would be to use Aspect Oriented Programming.

The interesting solution is to reframe logging as an instance of the Writer Monad. Here's an example in Scala: http://blog.tmorris.net/posts/the-writer-monad-using-scala-example/. Note: the concept is language-agnostic, Scala is simply used as a convenient notation here.

Jörg W Mittag
  • 101,921
  • 24
  • 218
  • 318