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

Utilize protocol to get defaults (set-builder-fn, table-fn, label-fn and etc) #49

Closed
kirillsalykin opened this issue Jul 24, 2019 · 9 comments

Comments

@kirillsalykin
Copy link

kirillsalykin commented Jul 24, 2019

Is your feature request related to a problem? Please describe.
It is pretty annoying to pass custom set-builder/table-fn/label-fn to every query or statement.

Describe the solution you'd like
It would help a lot if there would be a way to specify it only once.
But having the possibility to pass override via opts still important.

Describe the solution you'd like
Maybe having a protocol to get defaults would help with this?
And if one wants to override defaults - it can be achieved with extend-protocol.

Similar to ReadableColumn approach.

@seancorfield
Copy link
Owner

Given that protocols are based on types I don't see how this would work -- could you elaborate on what you have in mind?

@kirillsalykin
Copy link
Author

kirillsalykin commented Jul 24, 2019

Sorry for not being clear.

I propose to create a new protocol:

(defprotocol NamingConvention
  (builder-fn [this])
  ...)

Then extend-protocol to all types which extended by Executable:

(extend-protocol p/Executable

  • java.sql.Connection
  • javax.sql.DataSource
  • java.sql.PreparedStatement
(extend-protocol p/NamingConvention
  java.sql.Connection
  (builder-fn [_] 
     as-maps)

   javax.sql.DataSource
   (builder-fn [_] 
      as-maps)

   java.sql.PreparedStatement
  (builder-fn [_] 
     as-maps))

And get default builder-fn using protocol

(let [builder-fn (get opts :builder-fn as-maps)

builder-fn (get opts :builder-fn (p/builder-fn this)

If one wants to override it - he should extend-protocol to the java(x).sql types.

A bit annoying, but doable.

What do you think?

@kirillsalykin
Copy link
Author

Hm, not sure it works...

@seancorfield
Copy link
Owner

I think you could base it on ResultSet -- I think everywhere that "gets" the :builder-fn option has access to a rs object...?

@kirillsalykin
Copy link
Author

I'll play more with to ensure it actually works.
Will get back to you.

Thanks!

@seancorfield
Copy link
Owner

So you're thinking that the library would provide a default implementation for Object and then users could override it for ResultSet with their defaults instead?

I still feel this is really hacky...

@kirillsalykin
Copy link
Author

I agree, feels hacky, but cant think of any other approach to override default behavior w/o global variable.

@seancorfield
Copy link
Owner

I canvassed some of Clojure contributors and everyone who had an opinion felt this was a bad approach. Protocols should really only be extended either by the code that owns the protocol (next.jdbc in this case) or owns the type -- and the connection to ResultSet was felt to be tenuous at best.

So I'm closing this out. My original suggestion of creating a wrapper API still stands -- but I think that should be in user code.

@kirillsalykin
Copy link
Author

I agree.

Thanks for investigating it.

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