Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have code like this (pseudocode)

foreach(Box box in boxes)
{
    if(boxFilter.PassesFilter(box))
    {
      // do something useful
    }
    else
    {
      Log.Log(format("Box %s was rejected", box.BoxId))
    }
}

But now I'm thinking I'd like the log message to have more information such as "Box 1 rejected because too tall" or 'Box 549 rejected: 25" height exceeds Max height of 24"'.

What's the best idiom to get this information into the log? Do I have to create a FilterResult object that is returned from the PassesFilter method? Or I guess I could also throw an exception and catch it and log it? Or something else?

I happen to be using C++.

Update

In addition to logging I might want to also present a message to the user in the UI.

share|improve this question
add comment

3 Answers

I can see two main options here; an enum could be returned, or you could use an exception.

Personally, I would use an exception, but I'm not an experience C++ programmer, so I'm not sure if it fits the common idiom of the language.

share|improve this answer
add comment

Ultimately, logging is instrumentation; something a bit more humane than debugging symbols or profiling instrumentation; the logging should not have an effect on program flow, it should only faithfully report on it.

Do I have to create a FilterResult object that is returned from the PassesFilter method?

If you want to have logging at that level, then it should be the PassesFilter method that actually logs a message. To associate the filter result with a particular box, just emit a log message with the box identifier just before (or after) the filter method is called. It is then up to the logging system to associate the two messages and do something fancy with them, or not.

Or I guess I could also throw an exception and catch it and log it?

I wouldn't do this; At least, not for the purposes of logging. Exceptions are great, if they represent meaningful program flow. It's also OK to instrument exceptions in a log file, but the step of creating an exception for the purposes of instrumentation is not going to make your code any better.

bool BoxFilterClass::PassesFilter(const Box & box){
    if (box.height > BoxFilterClass::MAX_HEIGHT) {
        Log.Debug(format("Box %s to tall, expected %s, actual: %s", box.id, BoxFilterClass::MAX_HEIGHT, box.height));
        return false;
    }
    else if (box.weight > BoxFilterClass::MAX_WEIGHT) {
        Log.Debug(format("Box %s to heavy, expected %s , actual: %s", box.id, BoxFilterClass::MAX_WEIGHT, box.weight));
        return false;
    }
    // and so on
    return true;
}
share|improve this answer
    
I guess I have resistance to logging in the Filter class as I'm trying to keep the logging as a concern of the consuming class while keeping the Filter class "pure". And it's possible this goal is not justified but that's what's in my head. As another example, how about if instead of logging I wanted to present a message to the user, would you still suggest the same type of solution? –  User Mar 9 '12 at 2:45
    
Logging is two separate concerns, "What is happening" and "Who cares?", The "Who Cares" bit is hard to predict, and you should take pains not to make assumptions about it. To be useful, though, logging should address the "What's happening" part to the highest detail, and as close to where the action is as is possible. –  TokenMacGuy Mar 9 '12 at 3:07
add comment

For these situations, You can come up with a Response class which has a list of warnings, errors and information messages.

In this way, You can keep on adding errors to this response and return it back to the caller.

Also you dont have to add code in all the methods for logging errors. In a higher level(facade caller), you can add a check that if the response has errors or warnings, You can log them all.

Something like this (pseudo)

public class Response {

List<String> errors;
List<String> warnings;
List<String> infos;

}

In your code

foreach(Box box in boxes)
{
    if(boxFilter.PassesFilter(box))
    {
      // do something useful
    }
    else
    {
      response.addError(format("Box %s was rejected", box.BoxId))
      return response;
    }
}

--- In a higher level for all callers

If(response.hasErrors or warnings) {

logger.log(response.getErrors());
logger.log(response.getWarnings());


}
share|improve this answer
    
If I understand you correctly I think you're solving a slightly different problem than the one I have. It looks like you're talking about aggregating errors in a for loop. Instead, my question is about the fact that only the PassesFilter method has details about what happened and how does it communicate that back to the caller so the caller can log it. –  User Mar 9 '12 at 22:46
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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