0

I have a bunch of entries like these two:

        if (update) {
            if (activity.getName() == null) {
                logger.debug("      Setting name on " + id);
            } else
            if (!activity.getName().equals(name)) {
                logger.debug("      Updating name on " + id);
            }
        }
        // if (!update) not logged on purpose
        activity.setName(name);

        if (update) {
            if (activity.getPlannedDuration() == null) {
                logger.debug("      Setting plannedDuration on " + id);
            } else
            if (!activity.getPlannedDuration().equals(duration)) {
                logger.debug("      Updating plannedDuration on " + id);
            }
        }
        // if (!update) not logged on purpose
        activity.setPlannedDuration(duration);

and for code readability purposes I'd like to replace them with something like this:

        updateField(update, name, "name", activity.getName, activity.setName);
        updateField(update, duration, "plannedDuration", activity.getPlannedDuration, activity.setPlannedDuration);

I'm aware this is a common question, I did my homework, and wraping methods to Callable interface seems to be the simplest solution. But, that solution would be even more of a mess than my current code (remember, I'm doing this for the readability).

So, is there an elegant solution for my problem in Java?

Community
  • 1
  • 1
dijxtra
  • 2,681
  • 4
  • 25
  • 37

1 Answers1

1

Well you could refactor that particular code to:

logUpdate(update, activity.getName(), name, "name", id);
activity.setName(name);

logUpdate(update, activity.getPlannedDuration(), plannedDuration,
          "planned duration", id);
activity.setPlannedDuration(plannedDuration);

...

static void logUpdate(boolean update, Object currentValue,
                      Object newValue, String field, String id) {
    if (currentValue == null) {
        logger.debug("      Setting " + field + " on " + id);
    } else if (!currentValue.equals(newValue)) {
        logger.debug("      Updating name on " + id);
    }    
}

It's not fabulous, but it's still an improvement. Note that currently you're actually updating the field whether or not you logged about it - are you sure that's what you intended? I'd more expect something like:

if (update) {
    logUpdate(activity.getName(), name, "name", id);
    activity.setName(name);

    logUpdate(activity.getPlannedDuration(), plannedDuration,
             "planned duration", id);
    activity.setPlannedDuration(plannedDuration);
}

But no, there's no trivial way of passing methods around in Java at the moment. Java 8 will make it much simpler with method references and lambda expressions, however.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Yeah, I'm aware I do not log all options, I added a clarification to my code. I thought about separating logging and setting, but I thought I'd ask here first for a complete solution. Anyway, thanks for the insight! – dijxtra Oct 01 '13 at 06:15
  • @dijxtra: No, you've missed my point. You're updating the object (calling `setName`) even if `update` is false. Surely that's not appropriate. – Jon Skeet Oct 01 '13 at 06:18
  • Yup, it is appropriate. If update is false, that just means I'm not updating the field on an existing activity, but creating a new activity altogether (which was written to the log previously), and therefore it is obvious that all those setters need to be called. Therefore, there's no need to log that. – dijxtra Oct 01 '13 at 06:24