1

I have a public method which calls a private method, which conditionally calls another private method. All three methods return a boolean primitive value, however the value is sometimes changing from what I expect it to be.

I am certain I am missing something incredibly obvious, but I am now too close to the code to see the problem.

Here is a cleaned up sample version of the code and the results I am seeing:

EDIT: I have added the ACTUAL code at the bottom

public class MyClass { 

    // Default Constructor
    public MyClass() {}

    public boolean foo(final Object arg0, final Object arg1, final Object arg2) { 
        final boolean result = this.bar(arg0, arg1, arg2); 
        System.out.println("foo: " + result);
        return result;
    } 


    private boolean bar(final Object arg0, final Object arg1, final Object arg2) { 
        boolean result = false;
        System.out.println("bar: " + result); 

        try {  
            // complicated code that could generate an exception

            if (this.wowsers(arg0, arg1, arg2)) {
                result = true;
                System.out.println("bar: " + result); 
                System.out.println("bar: call to wowsers() returned true"); 
            }

        } catch (Exception e) { 
            System.out.println("SOMETHING BLEW UP");
            e.printStackTrace();
        } finally { 
            // This should NOT change result
            this.irrelevantMethod_1(null);
            this.irrelevantMethod_2(null);
            this.irrelevantMethod_3(null);
        } 

        System.out.println("bar: " + result); 
        return result;
    } 

    private boolean wowsers(final Object arg0, final Object arg1, final Object arg2) { 
        boolean result = false;

        // complicated code involving the passed in arguments
        // this MIGHT change result

        System.out.println("wowsers: " + result); 
        return result;
    }

    private void irrelevantMethod_1(Object arg0) { 
        // Nothing in here to change result 
    } 

    private void irrelevantMethod_2(Object arg0) { 
        // Nothing in here to change result 
    } 

    private void irrelevantMethod_3(Object arg0) { 
        // Nothing in here to change result 
    } 
} // END Class MyClass

Calling Code:

MyClass myInstance = new MyClass(); 
myInstance.foo(); 

Console Output:

> bar: false
> wowsers: true 
> bar: true
> bar: call to wowsers() returned true 
> foo: false

In the sample above, the value of result gets set to true in method dowsers(), and is correctly returned to bar(). When bar() tests the returned value from wowsers() and finds it to be true it sets it's own result to true and prints to the console the value of result and prints that dowsers() returned a true.

The System.out.println at the end of bar() is NEVER executed (at least I never see anything show up in the console), and the value of result (which at this point is true) is returned as false.

The foo() method then prints the value it received from the call to bar, which is always false.

Does anybody see where I'm messing up?

EDIT: Here is the ACTUAL code -replaces methods foo() and bar()

public boolean sendReminderNotifcation(final RequestController controller, final Session session, final Request request,
        final Comment comment, final boolean treatAsNewNote) {

    final boolean result = this.sendNotification(controller, session, request, comment, null, MailType.Remind, treatAsNewNote);
    System.out.println("sendReminderNotifcation(): " + result);
    System.out.println("***");
    return result;
}



private boolean sendNotification(final RequestController controller, final Session session, final Request request,
        final Comment comment, final TreeSet<String> copyto, final MailType mailtype, final boolean treatAsNewNote) {

    HashMap<NotifyWhom, TreeSet<String>> sendTo = null;
    boolean result = false;

    System.out.println("sendNotification(): " + result);

    try {
        if (null == controller) { throw new IllegalArgumentException("RequestContoller is null"); }

        this.setRequestURLprefix(controller.getRequestURLprefix());

        if (null == session) { throw new IllegalArgumentException("Session is null"); }
        if (null == request) { throw new IllegalArgumentException("Request is null"); }
        if (null == mailtype) { throw new IllegalArgumentException("mailtype is null"); }

        final EnumSet<NotifyWhom> recipients = this.getRecipients(controller, session, request, mailtype, treatAsNewNote);

        if ((null == recipients) || recipients.isEmpty()) { return false; }

        final HashMap<NotifyWhom, TreeSet<String>> tempSendTo = this.getSendTo(controller, request, recipients);
        if (null == tempSendTo) { throw new RuntimeException("NO RECIPIENTS FOR NOTIFICATION"); }

        // clear out any duplicates
        sendTo = this.purgeDuplicateRecpients(tempSendTo);

        // Update Prior Assignee Information
        // Update Requestor Information
        // Update Queue Owner Information
        this.updateReOpenedInformation(controller, session, request);

        final String subject = (request.isReOpened()) ? HelpdeskNotifications.SUBJECT_REOPENED : HelpdeskNotifications.SUBJECT_UPDATED;

        final Iterator<Entry<NotifyWhom, TreeSet<String>>> sit = sendTo.entrySet().iterator();
        final TreeSet<NameHandle> exclude = this.getExcludeRecipients();
        while (sit.hasNext()) {
            final Map.Entry<NotifyWhom, TreeSet<String>> entry = sit.next();
            final MailNotifyKey key = new MailNotifyKey(this.getLogger(), mailtype, entry.getKey());
            if (MailType.Remind.equals(mailtype)) {
                final Status status = request.getStatus();
                final MailRecipientOption mro = (null == status) ? null : status.getMailRecipientOption(key);

                // A null mro indicates that Notifications are DISABLED
                if (null == mro) { return false; }
            }

            final TreeSet<String> sendto = entry.getValue();
            if (this.sendEmail(controller, session, request, subject, comment, sendto, copyto, exclude, key, treatAsNewNote)) {
                result = true;
                System.out.println("sendNotification(): " + result);
                System.out.println("sendNotification(): (call to sendEmail() returned true)");
            }
        }

        // Send Special Re-Opened Notifications
        if (this.sendReOpenedNotifications(controller, session, request, subject, comment, treatAsNewNote)) {
            result = true;
            System.out.println("sendNotification(): " + result);
            System.out.println("sendNotification(): (call to sendReOpenedNotifications() returned true)");
        }

    } catch (final Exception e) {
        this.getLogger().logException(this, e);
        e.printStackTrace();
    } finally {
        this.setPriorAssigneeNotify(null);
        this.setReOpenedRecipients(null);
        this.setExcludeRecipients(null);
    }

    System.out.println("sendNotification(): " + result);
    return result;
}

The Console Output I am getting is:

> sendNotification(): false 
> sendNotification(): true 
> sendNotification(): (call to sendEmail() returned true) 
> sendReminderNotifcation(): false 
> ***

(I realize I should have posted the ACTUAL code in the first place)

For some reason I cannot figure out, it seems as though the final two lines of code in method bar() and method sendNotification() do not appear to be running. Is there some other way for a method to complete and return that I am unaware of?

spanky762
  • 171
  • 1
  • 11
  • the third `bar + result` log is not included in your console output, but there is nothing suggesting it could be skipped, since there is no early return, nor catch block that would allow `foo` log to be executed alone. – njzk2 Oct 22 '14 at 16:04
  • Yes, exactly. That is one of the things I can't figure out. It should be showing up, but it is not. I have checked and checked and rechecked to make sure there is no other return statement in the method, and cannot find one. – spanky762 Oct 22 '14 at 16:05
  • something is definitely missing from the code you pasted. – njzk2 Oct 22 '14 at 16:06
  • did you run that step-by-step in a debugger? – njzk2 Oct 22 '14 at 16:06
  • My guess is that you're in fact returning something from the finally block. you should provide a complete, reproducible test case. – JB Nizet Oct 22 '14 at 16:07
  • No, I have not run it step-by-step in a debugger, the code is running in a scheduled process on JSF server and I don't have a server-debugger set up. – spanky762 Oct 22 '14 at 16:28

3 Answers3

1

I suggest you put a debug print statement before this line:

              if (null == mro) { return false; }

Since this is in a while loop, and allows the method to return false even thoughresult has been set true. I'll bet this is where the false return is coming from, and why you don't see the final print statements being executed.

davmac
  • 20,150
  • 1
  • 40
  • 68
  • BINGO. Thanks for nailing this. – spanky762 Oct 22 '14 at 18:01
  • how does being in a while loop allow it to return false even tho it is true? – Joza100 Jun 01 '18 at 17:23
  • @Joza100 It is nothing to do with the fact that it's in a while loop. There is nothing special about the `result` variable and the statement says `return false` rather than `return result`. If you specifically have a statement which says `return false` then it will do so. The fact that it is in a loop explains why the call to `sendEmail` does execute (and return true) before the `sendNotification` method itself returns false - because that happens on the 2nd iteration. – davmac Jun 04 '18 at 10:58
0

Read this :

Is Java "pass-by-reference" or "pass-by-value"?

In this method

private boolean wowsers(final Object arg0, final Object arg1, final Object arg2) { 
    boolean result = false;

    // complicated code involving the passed in arguments
    // this MIGHT change result

    System.out.println("wowsers: " + result); 
    return result;
}

You re-instantiate boolean varaible. This is a new boolean result outside of scope method bar

Simply block. Just think.

            result = true;
            System.out.println("bar: " + result); 

And finally it looks like a

private boolean bar(final Object arg0, final Object arg1, final Object arg2) { 
    boolean result = false;
    // no reassignee result value
    return result; 
}

so you return false

Community
  • 1
  • 1
Sergey Shustikov
  • 15,377
  • 12
  • 67
  • 119
  • Thank you for your answer. I have updated the original question to include the actual code. I'm not sure if I fully understand your answer in that I am not passing the value of result into the methods, just returning it. The scope should be local and independent inside of each method, and the value returned. Please explain if I am misunderstanding here. – spanky762 Oct 22 '14 at 16:26
  • try to add **synchronized** to both methods modificators. – Sergey Shustikov Oct 22 '14 at 16:47
0
final boolean result

This means you want result will never change after its first value.

And that's what you get. Remove final.

Vouze
  • 1,678
  • 17
  • 10
  • final boolean result is intentional, it is there as an in method variable simply so that I could log the result of the function call. It is not intended to be changed once set, hence the reason for final. – spanky762 Dec 01 '14 at 22:37