Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executeScalarList should use converters #36

Closed
aaberg opened this issue Jan 14, 2014 · 4 comments
Closed

executeScalarList should use converters #36

aaberg opened this issue Jan 14, 2014 · 4 comments
Assignees
Milestone

Comments

@aaberg
Copy link
Owner

aaberg commented Jan 14, 2014

executeScalarList should use registered converters.

@ghost ghost assigned aaberg Jan 14, 2014
@aldenquimby
Copy link
Contributor

I have a possible suggestion here.

Maybe you could simplify the interface by having executeAndFetch and executeAndFetchFirst handle cases where someone is asking for a scalar. That way you wouldn't need a separate method to handle scalars, and users wouldn't need to think about which method to call depending on their type - they would just always call the generic execute and fetch methods.

For backwards compatibility, you could keep executeScalar and executeScalarList and have them just call executeAndFetch.

Looking at the existing code, I think this could work by checking if PojoMetadata.propertySetters is empty. I would be happy to do this on a branch and show you what I mean if this is confusing.

@aaberg
Copy link
Owner Author

aaberg commented Feb 2, 2014

That is an interesting idea!

The only consequence I can think of, that might be a problem, is if there is a property on the class with the same name as the column in the database. In this case, sql2o will try to convert that property instead of using a converter on the class itself.

But if we can find a good way of handling this, I am very positive to the idea. What do you think?

@aldenquimby
Copy link
Contributor

Yeah good point. I think I have an idea for how it could work.

I'll try it out and submit a pull request for you to review if I can get it working

@aldenquimby
Copy link
Contributor

#46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants