4

I have the following code where the goal is that when a method is called on a parent it then calls the same method on all of it's children, this is then where the actual work is done.

public class Parent {

    Child[] children;

    public doWork() {
        for(int i = 0; i < children.size(); i++) //For all my children
            children.get(i).dowork();            //Call the same method
        Log.ReportWorkBeingDone(this);           //Report this has happened
    }
}

public class Child extends Parent{

    @override
    public doWork() {
        //Actual Work                            //Run the actual code
        Log.ReportWorkBeingDone(this);           //Report this has happened
    }
}

Right now when the doWork() is called on the parent the Log class gets a method call from the Parent and then once from each of the Child objects in it's array.

Is there a programming pattern or just a good way of only running the call to the Log class once? So if doWork() is called on a Child from outside this chain of classes then the child will call the Log method but if doWork() is called on the parent only the parent will call the Log method and the children won't?


All I can think of is having something like this:

public class Parent {

    Child[] children;

    public doWork() {
        sneakyHiddenWork();
        Log.ReportWorkBeingDone(this);                     //Report this has happened
    }

    protected sneakyHiddenWork(){
        for(int i = 0; i < children.size(); i++)           //For all my children
            children.get(i).sneakyHiddenWork();            //Call the same method
    }

}

public class Child extends Parent{

    @override
    protected sneakyHiddenWork() {
        //Actual Work                                      //Run the actual code
    }
}
Toby Smith
  • 1,505
  • 2
  • 15
  • 24
  • 1
    It is good as you wrote. I will just use name `doWork` and `doWorkAndLog` – ByeBye Dec 11 '17 at 00:17
  • 2
    I'm pretty sure this can be done with AspectJ, but I doubt that it's worth introducing new dependencies just for that. – Marvin Dec 11 '17 at 00:19
  • An alternative is to pass a `boolean isParent` into `doWork` and only do the logging if it's true. – Geoff Dec 11 '17 at 00:20
  • Checked LoggerFactory [https://www.slf4j.org/apidocs/org/slf4j/LoggerFactory.html ]? – riadrifai Dec 11 '17 at 00:20
  • 1
    Ditto you could new an Exception (but not throw it) and call its getStackTrace and inspect the output, but that's expensive. Agree this is OK as is, but pick a better name. If your logging function accepts `this` (and calls its getClass?) you may not even need to override doWork in the child. Also consider if you should log before or after you do the work. You could also use the Java 'for each', i.e. `for (Parent p : children) { p.sneakyHiddenWork(); }` rather than index and get. – Rup Dec 11 '17 at 00:21
  • Yeah I just threw names in for now and to be clear :) Also just realised I don't need the public overridden method in the `Child` do I... It's exactly the same as the one in the `Parent`. – Toby Smith Dec 11 '17 at 00:25
  • The technique whereby code behaves differently depending on who called it is called 'state orientation', and was identified as poor practice in the 1960s. Find a better design. – user207421 Dec 11 '17 at 01:14
  • Interesting. I knew it was poor practice, but didn't realize there was a name for it. – Stephen C Dec 11 '17 at 01:28
  • There is no good way of doing this, because a method should not care where it is called from. You have a code smell, or an XY Problem. – Raedwald Jan 02 '18 at 18:31

1 Answers1

2

Your solution (sneakyHiddenWork) is the best solution ... though your choice of names is missing the point. (It is not "sneaky". It is a common pattern to do this kind of thing.)

Yes, it is possible to find out what the name of the calling method is, but this is not well-supported in Java1, and the procedure is expensive. You instantiate an exception object, and then look at the exception's captured stack frames returned by Throwable::getStackTrace. (That's what logger frameworks like log4j do under the covers ...)

See also: Accessing caller information quickly2


1 - There is no specific "get the name of my caller" method. Furthermore, the method that I described is actually non-portable since it permitted for Throwable::getStackTrace to return an empty array of frames.

2 - The top answer alludes to a couple of non-portable alternatives. The Java 9 version using the StackWalker API is promising, but it will still be relatively expensive because StackWalker needs to snapshot the stack frames.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216