Query.bind implementation && behavior

[dimzon]

https://github.com/aaberg/sql2o/blob/master/core/src/main/java/org/sql2o/Query.java#L219

1. getDeclaredMethods - this means inheritance not work
2. absent static modifier check check for static modifier (is it "as designed" to allow static bean properties can be added to Query params?
3. absent private modifier check so adding a private property to bean will cause runtime exception (keep in mind you can share this bean with someone else, the bean code is not 100% under your control)
4. seems like you analyze method name and lowercase it just like java.beans.Introspector do - why not just reuse it?
5. you does not analyze fields so there are no way to pass single anonymous object in Dapper.NET style:

executeQuery("select @name, @age", new Object{
    int age = 31;
    String name = "dimzon"
})

some fixes will definitly cause backward compatibility breaks ;(
so I propose

1. keep old bind as is and mark it @Deprecated
2. write new method bindObject with proper implementation and behavior
3. to branch between "bean-style" and "dapper-style" use marker interface pattern ParamObject

// just empty interface
public interface ParamObject{}

executeQuery("select @name, @age", new ParamObject{
    int age = 31;
    String name = "dimzon"
})

[aldenquimby]

I haven't used the bind method, but it sounds to me like the behavior is incorrect. If that's the case, I don't think we need a new method, we should just fix the existing one.

Also, what's the benefit of the marker interface in this case over a plain Object?

[dimzon]

Also, what's the benefit of the marker interface in this case over a plain Object?

if (obj instanceof ParamObject){
  // analyze fields and put them as param values
} else {
  // analyze getXXXX methods (bean-style) and put them as param values
}

[aaberg]

The inheritance issue should be fixed. It is also described in issue #55.

When binding objects, we should only bind expected parameters. So if we have 2 parameters in the sql-statement, but 3 properties on the binded object, it should only bind the 2 parameters and ignore the last property. It seems like this behavior has been changed recently from the original implementation of bind(). as @dimzon points out, it will throw exception if this happens today! This should be fixed.

Given the above, I don't think the changes suggested will break backwards-compatibility?

If we add analysis of fields (in addion to getters), we don't need the ParamObject interface. Then it should work with:

executeQuery("select @name, @age", new Object(){
    int age = 31;
    String name = "dimzon!"
})

as well as with:

executeQuery("select @name, @age", myPojo)

[dimzon]

who will do this (I can but if anyone else want feel free to stop me)

[aaberg]

@dimzon - Your on a killing spree :)
Go ahead. I'm really glad that sql2o gets the love it deserves.

BTW, you deserve your name under developers in the pom.xml file (If you would like it there, of cause - no pressure :) You have been a really valuable contributor! Without you and @aldenquimby, sql2o would never have been able to become such an awesome project!

[aaberg]

btw - When fixing an issue like this. I would recommend creating a new branch just needed for the fix.

git branch fix-issue-80
git checkout fix-issue-80

Do your stuff, and check it in with a commentary that mensions the issue number, like this: "ref #80 - comment here".
when you are done:

git push -u origin fix-issue-80    // -u sets upstream to origin/fix-issue-80, so that the next time you push on the same branch, you can just write "git push"

On github, create a pull request from the new branch.

This way you can start on working on other things, and not have to wait for me to merge in your work.