Improvements to static method matching

[Stuart Miller]

Hello,

Raymond and I have just changed the static method matching to prefer methods with more parameters. This matches the logic used in constructor matching.

You can check out an example in the test.

Stuart

[Nick Barrett]

Hi Stuart and other yadicts, 

hey my first post!

Not sure if this is related, or even by design, but I've found that when a class in the container fails to get instantiated for some run-time reason, the following container exception is thrown. To me this seems misleading as my class does have the appropriate public static method:

com.googlecode.yadic.ContainerException: <my class> does not have a satisfiable public static method

at com.googlecode.yadic.resolvers.StaticMethodResolver.resolve(StaticMethodResolver.java:52)

at com.googlecode.yadic.resolvers.Resolvers$2.resolve(Resolvers.java:54)

at com.googlecode.yadic.resolvers.LazyResolver.resolve(LazyResolver.java:31)

at com.googlecode.yadic.resolvers.Resolvers.resolve(Resolvers.java:117)

at com.googlecode.yadic.BaseTypeMap.resolve(BaseTypeMap.java:37)

at com.googlecode.yadic.DelegatingTypeMap.resolve(DelegatingTypeMap.java:49)

at com.googlecode.yadic.resolvers.Resolvers.resolve(Resolvers.java:117)

at com.googlecode.yadic.SimpleContainer.get(SimpleContainer.java:36)

at <my stack>

Caused by: java.lang.reflect.InvocationTargetException

at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)

I would have thought that yadic should be able to distinguish between a failure to instantiate due to constructor matching problems vs runtime error? I'm on the latest version 158

Cheers

Nick

Any thoughts?

[Raymond Barlow]

Not to be a stickler, but what it is telling you is most likely correct. Can we see your code? We only need the constructors & public static factory methods, and the yadic code you are using to register/construct.

/Raymond

--
You received this message because you are subscribed to the Google Groups "yadic" group.
To unsubscribe from this group and stop receiving emails from it, send an email to yadic+un...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Nick Barrett]

Hey Raymond, 

Sorry - I wasn't being secretive about my code, just forgot I posted this question!

Okay. The class that is blowing up is NotificationMessageFlowQueryService. Note that everything works fine if there isn't a runtime problem so it's not a basic problem with constructor matching.

 1 package me.v.service.tm;
 2 
 3 import com.googlecode.yadic.Container;
 4 import com.googlecode.yadic.Containers;
 5 import me.v.service.query.NotificationMessageFlowQueryService;
 6 import me.v.util.logging.LogReader;
 7 import me.v.util.logging.LogReaderService;
 8 
 9 public class NotificationMessageFlowQueryServiceCreator {
10 
11     public static NotificationMessageFlowQueryService create(String userName) {
12         Container container = Containers.container();
13         container.addInstance(String.class, userName);
14         container.addInstance(LogReader.class, LogReaderService.create());
15         container.add(NotificationMessageFlowQueryService.class);
16         return container.get(NotificationMessageFlowQueryService.class);
17     }
18 
19 }

Stack trace is shown below - you can see that problem is happening when NotificationMessageFlowQueryService is retrieved from the container.

 

com.googlecode.yadic.ContainerException: me.v.service.query.NotificationMessageFlowQueryService does not have a satisfiable public static method at com.googlecode.yadic.resolvers.StaticMethodResolver.resolve(StaticMethodResolver.java:52) at com.googlecode.yadic.resolvers.Resolvers$2.resolve(Resolvers.java:54) at com.googlecode.yadic.resolvers.LazyResolver.resolve(LazyResolver.java:31) at com.googlecode.yadic.resolvers.Resolvers.resolve(Resolvers.java:117) at com.googlecode.yadic.BaseTypeMap.resolve(BaseTypeMap.java:37) at com.googlecode.yadic.DelegatingTypeMap.resolve(DelegatingTypeMap.java:49) at com.googlecode.yadic.resolvers.Resolvers.resolve(Resolvers.java:117) at com.googlecode.yadic.SimpleContainer.get(SimpleContainer.java:36) at me.v.service.tm.NotificationMessageFlowQueryServiceCreator.create(NotificationMessageFlowQueryServiceCreator.java:16) at me.v.service.tm.NotificationMessageFlowQueryServiceCreatorTest.testCreate(NotificationMessageFlowQueryServiceCreatorTest.java:9)

[Raymond Barlow]

Can you also post the NotificationMessageFlowQueryService code? Feel free to remove everything except the constructors and static factory methods.

/Raymond

[Nick Barrett]

Yo Raymond,

Here is the constructor for that service. Note that buildNotificationUserJourneys() could throw an exception. Perhaps that's bad design?

 1 
 2     public static NotificationMessageFlowQueryService create(final String userName, LogReader logReader) {
 3         return new NotificationMessageFlowQueryService(userName, logReader);
 4     }
 5 
 6     private NotificationMessageFlowQueryService(final String userName, LogReader logReader) {
 7         this.userName = userName;
 8         this.logReader = logReader;
 9         buildNotificationUserJourneys();
10     }

[Raymond Barlow]

I think I know what is happening.

Yadic _is_ actually finding the factory method, but when it invokes it, your exception is thrown. This manifests as a container exception. If you need to get to your exception, you can catch drill down through causes.

I put together your bits of code below. This example shows what I believe is happening to you. If you remove the exception from buildNotificationUserJourneys it constructs fine.

Does that explain it?

/Raymond

 1 public class NotificationMessageFlowQueryServiceCreator {
 2     public static void main(String[] args) {
 3         create("bob");
 4     }
 5 
 6     public static NotificationMessageFlowQueryService create(String userName) {
 7         Container container = Containers.container();
 8         container.addInstance(String.class, userName);
 9         container.addInstance(LogReader.class, LogReaderService.create());
10         container.add(NotificationMessageFlowQueryService.class);
11         return container.get(NotificationMessageFlowQueryService.class);
12     }
13 
14     public interface LogReader { }
15 
16     public static class LogReaderService implements LogReader {
17         public static LogReaderService create() {
18             return new LogReaderService();
19         }
20     }
21 
22     public static class NotificationMessageFlowQueryService {
23         private final String userName;
24         private final LogReader logReader;
25 
26         public static NotificationMessageFlowQueryService create(final String userName, LogReader logReader) {
27             return new NotificationMessageFlowQueryService(userName, logReader);
28         }
29 
30         private NotificationMessageFlowQueryService(final String userName, LogReader logReader) {
31             this.userName = userName;
32             this.logReader = logReader;
33             buildNotificationUserJourneys();
34         }
35 
36         private void buildNotificationUserJourneys() {
37             throw new RuntimeException("cheese");
38         }
39     }
40 }

[Nick Barrett]

Yup this makes sense Raymond and were my findings.  But I thought that is was misleading that yadic throws the X does not have a satisfiable public static method exception, given that it seems commonplace for a constructor to be able to throw an exception -> http://stackoverflow.com/questions/1371369/can-constructors-throw-exceptions-in-java

I would have a different exception type would be more appropriate to differentiate from the basic constructor matching problem.

Cheers

Nick

[Raymond Barlow]

You could argue that, but I feel the exception message is adequate. Yadic prioritises constructors by arity descending. It the calls each constructor in turn until one works without exception.

So in yadic's world, it needs a public method that doesn't throw, to satisfy it's contraints-thus the message 'could not find SATISFIABLE....'.

And as mentioned, you still can get to the exception that was thrown by your app--so, nothing lost.

/Raymond

—
Sent from Mailbox for iPhone

[Daniel Worthington-Bodart]

+1 

A constructor by definition is not satisfiable if it throws an exception, it's generally considered good practice for constructors to not actually do anything other than syntactic validation (Illegal argument, Null check etc)

[Nick Barrett]

okay guys ... you win!! 

I'll change my code to keep the framework happy :-)

[waits for retorts]

[Matt Savage]

+1 that constructors shouldn't throw exceptions.

Having said that, I've accidentally written constructors/factory methods that throw exceptions before and spent a couple of minutes scratching my head about exactly this exception.

In terms of devexp, is there a way we could change the wording slightly to suggest checking the root exception for what actually went wrong? Not this, but something like this:

"Could not resolve XXX- please check root exception for details."

[Daniel Worthington-Bodart]

Sounds good

[Matt Savage]

Done

[Raymond Barlow]

Really? I would think that every java dev should JUST do this anyway. That is, look down through the exception messages.

It feels a little redundant to me.

/Raymond

—
Sent from Mailbox for iPhone

[Matt Savage]

"Should" != "Will" ;)

Interesting that me and Nick both did the same thing. And I've got no excuse because I'd read all the source code at the time and "should" have known how it worked.

I guess it's because 99% of the time the cause of the exception is that a dependency isn't in the container. The code naturally conditions me into believing that the exception means I've forgotten to add a dependency.

In my case, I did look down the stack, but initially not to the bottom, for some reason. I think I jumped to conclusions about what dependency was missing and then it took a while to return to looking at the stack trace after that. It was only 5 or 10 minutes of my life, but they were annoying minutes.

It's similar to the utterlyidle thing where

Path("/resource")
class Resource{}

Didn't work because of the leading slash. Total programmer error, granted, but we can be friendly about it, so why not?

Honestly, I'm not sure that changing the message will make much difference- people's brains will probably still associate the stack trace with a missing dependency. But it seems nice to try, no?

A clean 30 second change that might save (5 minutes * no. of users) seems worth it?

I'm aware this change doesn't warrant this degree of discussion, but the general topic of devexp is interesting.

[Raymond Barlow]

I guess for me I think more about the experienced user rather than the noobs. 

I value

* succinct, with no duplication in the long term

over

* verbose problem solvey  (maybe with some duplication) in the short term

Yes I might spend a bit of time figuring it out first time--but at least I'll get a greater understanding.

/Raymond

—
Sent from Mailbox for iPhone

[Matt Savage]

Are me and Nick included in the n00bs?

Agree on your prioritisation of values, where there's a conflict. In this case we're just changing an error message.

[Raymond Barlow]

Noobs just to mean, haven't seen this problem before (not trying to be mean).

Happy with message change--like you said, am enjoying the chat :)

—
Sent from Mailbox for iPhone

[Matt Savage]

Noobs just to mean, haven't seen this problem before (not trying to be mean).

Ah, I see what you mean. Yes, it's definitely true that I/we am/are unlikely to get stuck in the same place again. Why get stuck once though, is what I'm thinking.

I just love a library with really smooth corners (like you say, as long as they don't come with a long-term cost).

 

Happy with message change--like you said, am enjoying the chat :)

Cool cool. Me too.