Skip to content

Proposed new API design principles #83

New issue
New issue
Closed as not planned
Closed as not planned
Proposed new API design principles#83
Milestone
2.0

Description

@dimzon
dimzon
opened on Apr 8, 2014

I want to propose to re-design new (2.0) release version architecture|api using following principles

  1. Separation of Concerns - each component must solve 1 concrete task, every component can be replaced by custom implementation
  2. Using well-known GoF patterns instead self-made solutions (for example PojoMetadata is awful, it must be separated by PojoMetadataProvider && PojoMetadata)
  3. Must not prevent to write plain-old JDBC-code (must be able to get Connection from sql2o.Connection, PreparedStatement from Query, ResultSet from IterableResultSet (since we can use third-party library wich consumes JDBC objects, as example - converting ResultSet to CSV)
  4. Must be able to consume JDBC objects at any step (must be create sql2o.Connection from Connection, Query from PreparedStatement, must be able to transform ResultSet to all possible forms (List|Table|LazyTable|IterableResultSet (since sometimes we write not project-from-scratch but code wich obtains JDBC objects outside)

Activity

  1. aaberg
    added this to the 2.0 milestone on Apr 8, 2014

  2. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    So let's start...

    sql2o.Conf class.

    Aggregates all sql2o settings at one place. Implements sql2o.ConfSource.

    sql2o.ConfSource interface.

    contains ``sql2o.Conf getConf()` method

    sql2o.Sql2o class.

    my vision: it's factory class.
    responsibility: single entry-point, configure once use multiple. Takes javax.sql.DataSource in constructor. Produce sql2o.Conn. Also can create sql2o.Conn from java.sql.Connection, implements sql2o.ConfSource.

    typical usage: one instance per javax.sql.DataSource. Developer must create new instance and perform application-wide tweaks (Quirks|Strategies etc). In case of application servers developer can|must somehow implement singleton behavior (lifetime management is outside sql2o.Sql2o responsibility) using any preferred way (plain old static fields|IoC containers|whatever)

  3. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    sql2o.Conn class

    my vision: it's adapter class.

    responsibility: database connection. takes sql2o.ConfSource and java.sql.Connection in constructor. Can create sql2o.Query. Implements sql2o.ConfSource

    typical usage: mostly like plain old java.sql.Connection. Provides some syntax sugar. Works like a decorator around sql2o.ConfSource. Allows connection-wide tweaks (locally overriding tweaks from sql2o.ConfSource)

    JDBC consumer - can be created from plain JDBC Connection
    JDBC provider - can return ready-to use JDBC Connection

  4. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    sql2o.Query class

    my vision: it's PreparedStatement on steroids. takes sql2o.ConfSource, java.sql.Connection and sqlQuery in constructor
    can produce sql2o.QueryResult.

    typical usage: mostly like plain old java.sql.PreparedStatement.

    JDBC consumer - can be created from plain JDBC Connection
    JDBC provider - can return ready-to use PreparedStatement/CallableStatement

  5. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    sql2o.QueryResult class

    my vision: adapter around java.sql.ResultSet. takes java.sql.ResultSet and sql2o.ConfSource in constructor. Contains all neat List<Bean> listOf(Class<Bean> ofBeanClass) etc methods.
    as addition must provide generic getString, getInteger, getDate, getUUID etc methods which behavior is customizable via Quirks (some projects|databases use getNString another getString etc)

    JDBC consumer - can be created from plain JDBC ResultSet

  6. aaberg commented on Apr 10, 2014

    @aaberg
    aaberg
    on Apr 10, 2014
    Owner

    Separation of Concerns - each component must solve 1 concrete task, every component can be replaced by custom implementation

    I agree

    Using well-known GoF patterns instead self-made solutions (for example PojoMetadata is awful, it must be separated by PojoMetadataProvider && PojoMetadata)

    I'm not sure I agree that it is awful! :). But I agree that it's better to use patterns under the GoF umbrella.

    Must not prevent to write plain-old JDBC-code (must be able to get Connection from sql2o.Connection, PreparedStatement from Query, ResultSet from IterableResultSet (since we can use third-party library wich consumes JDBC objects, as example - converting ResultSet to CSV)

    I think this is a really good idea.

    Must be able to consume JDBC objects at any step (must be create sql2o.Connection from Connection, Query from PreparedStatement, must be able to transform ResultSet to all possible forms (List|Table|LazyTable|IterableResultSet (since sometimes we write not project-from-scratch but code wich obtains JDBC objects outside)

    As above. It think this is a really good idea :)

  7. aaberg commented on Apr 10, 2014

    @aaberg
    aaberg
    on Apr 10, 2014
    Owner

    I think your vision for sql2o v2 is very promising. I have read though your comments, and I think it looks very promising!

    Some things I think is important:

    • API must be clean and simple.
    • No annotations in core lib.
    • No manipulation of sql in core lib.
    • No need to catch any exceptions. All exceptions should be runtime exceptions.
    • I think the api today (as seen from the user) is good, some changes will happen, but it shouldn't change too much

    I think all of this fits nicely into your vision!

    Something I would like to discuss:

    • Will sql2o v2 have converters? How will they fit in?
    • How will transactions fit in? I think a transaction should be attached to a connection object and not extend it. I think it should be possible to begin a transaction both from a sql2o instance directly and from a connection instance.
    • Today you can execute a query directly on a sql2o instance (will automatically open a connection, execute the query, and close the connection). Should we keep this behavior?
    • v2 Will probably not compatible with v1. Should we create a new package (namespace) for sql2o v2, like they did with org.apache.commons.lang3. So an application can use both sql2o v1 and v2 at the same time?

  8. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    No need to catch any exceptions. All exceptions should be runtime exceptions.

    Yes, checked exception is wrost Java feature even. But re-throwing all of them as RuntimeException is evil too since you can't catch on concrete exception and you must search for proper CallStack every time...
    I just prefer to write throws Exception in all my business-code. This is really better solution - keep original exception clean without RuntimeException noise and Java doesn't harass you to catch anything.

  9. aldenquimby commented on Apr 10, 2014

    @aldenquimby
    aldenquimby
    on Apr 10, 2014
    Contributor

    I think the current design for sql2o exceptions is good and shouldn't
    change. Catch everything and rethrow as a Sql2oException, which extends
    RuntimeException. A lot of people don't add "throws Exception" everywhere,
    so it could be annoying to users to have to try catch every sql2o call

    On Thursday, April 10, 2014, Dmitry notifications@github.com wrote:

    No need to catch any exceptions. All exceptions should be runtime
    exceptions.
    Yes, checked exception is wrost Java feature even. But re-throwing all of
    them as RuntimeException is evil too since you can't catch on concrete
    exception and you must search for proper CallStack every time...
    I just prefer to write throws Exception in all my business-code. This is
    really better solution - keep original exception clean without
    RuntimeException noise and Java doesn't harass you to catch anything.

    Reply to this email directly or view it on GitHubhttps://github.com//issues/83#issuecomment-40145809
    .

  10. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    Will sql2o v2 have converters? How will they fit in?

    I think better way is

    interface ResultSetExtractor{
    Object extractValue(QueryResult resultSet, int columnNumber) throws SQLException;
}

    back direction is ParamSetter you already used

    current converters become Extractor|Setter factories

  11. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    Today you can execute a query directly on a sql2o instance (will automatically open a connection, execute the query, and close the connection). Should we keep this behavior?

    WARNING! IT'S IMHO
    API is something designed by professionals for non-professionals (read is some FrmeworkDesignGuide book). We must force consumers to use proper programming style. To learn to use try-with-resource. To not open new connection for every query.
    take PHP as example - it allows bad coding...

  12. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    v2 Will probably not compatible with v1. Should we create a new package (namespace) for sql2o v2, like they did with org.apache.commons.lang3. So an application can use both sql2o v1 and v2 at the same time?

    great idea

  13. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    How will transactions fit in? I think a transaction should be attached to a connection object and not extend it. I think it should be possible to begin a transaction both from a sql2o instance directly and from a connection instance.

    this is a subject to think about. this is not so easy, think about distributed transactions...
    I really like how this woks in .NET, let me show this:

    // using is 100% same as try-with-resource
using(Transaction tr = new Transaction()){
     // any code here automatically enlisted in transaction
     // this is done via ThreadLocal<Stack<Transaction>>
     // every constructor invocation puts new transaction onto stack top  
     // every dispose() (dispose == java AutoClosable.close() )
     // removes transaction from stack
     // every database statement constructor checks top of this stack
     // if it's not empty it will enlist in transaction
     this.doSomething(); 
     tr.commit();
}

  14. dimzon commented on Apr 10, 2014

    @dimzon
    dimzon
    on Apr 10, 2014
    ContributorAuthor

    A lot of people don't add "throws Exception" everywhere,
    so it could be annoying to users to have to try catch every sql2o call

    this is subject to talk about. I think (we can discuss and also look for best practics) wrapping everything in runtime exceptioon is evil (look my explanation in previous post). It's better to teach 'users' better and clean programming style (wich will help in other cases to, not just sql2o)

  15. aldenquimby commented on Apr 10, 2014

    @aldenquimby
    aldenquimby
    on Apr 10, 2014
    Contributor

    That is roughly how I use sql2o in my projects right now. I have a data
    layer package that wraps sql2o and drives a service/repository pattern.

    To be honest I don't think sql2o should implement this, because it's
    getting out of scope. The current version is one of the very few light
    weight object mappers in java. Imposing data layer patterns on users takes
    sql2o well beyond an object mapper, and I don't think that's the right
    move.

    Maybe someone could write an extension that keeps a stack of transactions
    and pushes/pops as things are created/disposed, but I really don't think
    that should be in the core.

    On Thu, Apr 10, 2014 at 6:03 PM, Dmitry notifications@github.com wrote:

    How will transactions fit in? I think a transaction should be attached to
    a connection object and not extend it. I think it should be possible to
    begin a transaction both from a sql2o instance directly and from a
    connection instance.

    this is a subject to think about. this is not so easy, think about
    distributed transactions...
    I really like how this woks in .NET, let me show this:

    // using is 100% same as try-with-resourceusing(Transaction tr = new Transaction()){
    // any code here automatically enlisted in transaction
    // this is done via ThreadLocal<Stack>
    // every constructor invocation puts new transaction onto stack top
    // every dispose() (dispose == java AutoClosable.close() )
    // removes transaction from stack
    // every database statement constructor checks top of this stack
    // if it's not empty it will enlist in transaction
    this.doSomething();
    rtr.commit();}

    Reply to this email directly or view it on GitHubhttps://github.com//issues/83#issuecomment-40148569
    .

  16. 15 remaining items

  17. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    Try public fields
    20.08.2015 11:24 пользователь "Antonio Conte" notifications@github.com
    написал:

    Hi Dimzon,
    my environment has public setter/getter with LOMBOK... but with a new try
    I've obtained that with some trick on fetchSize of DataSource I've obtained
    that with reflection I've 21s per 1M rows of 100bytes ca. and with Handler
    15s for same rows (obiouvsly of a simply query like select * from table
    where rownum < 1000000)

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  18. aconte76 commented on Aug 20, 2015

    @aconte76
    aconte76
    on Aug 20, 2015

    But public fields is not a good choice... for sure is faster but absolutely unsafe and unmaintanable... ;-)

  19. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    The bottleneck is not reflection since byte code used. Just one more
    virtual method call * 1M rows* n columns can take those 5 seconds
    20.08.2015 11:34 пользователь "Dmitry Alexandrov" dimzon541@gmail.com
    написал:

    Try public fields
    20.08.2015 11:24 пользователь "Antonio Conte" notifications@github.com
    написал:

    Hi Dimzon,
    my environment has public setter/getter with LOMBOK... but with a new try
    I've obtained that with some trick on fetchSize of DataSource I've obtained
    that with reflection I've 21s per 1M rows of 100bytes ca. and with Handler
    15s for same rows (obiouvsly of a simply query like select * from table
    where rownum < 1000000)

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  20. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    It depends. Until they are package private...
    20.08.2015 11:36 пользователь "Antonio Conte" notifications@github.com
    написал:

    But public fields is not a good choice... for sure is faster but
    absolutely unsafe and unmaintanable... ;-)

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  21. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    Btw I think we need a strategy to ignore setters and deserualize directly
    to private fields anyway. Your can achieve same effect now if you rename
    backing fields without touching setters and make column aliases in select
    to match fields...
    20.08.2015 11:36 пользователь "Antonio Conte" notifications@github.com
    написал:

    But public fields is not a good choice... for sure is faster but
    absolutely unsafe and unmaintanable... ;-)

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  22. aaberg commented on Aug 20, 2015

    @aaberg
    aaberg
    on Aug 20, 2015
    Owner

    Such a strategy should be optional and should not be enabled by default. it will be counter intuitive compared to ordinary java-standards. But I agree, that such a strategy could be nice to have, when you need a little extra performance for large datasets.

  23. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    Btw GSON use such strategy by default. Java builtin serialization too
    20.08.2015 12:04 пользователь "Lars Aaberg" notifications@github.com
    написал:

    Such a strategy should be optional and should not be enabled by default.
    it will be counter intuitive compared to ordinary java-standards. But I
    agree, that such a strategy could be nice to have, when you need a little
    extra performance for large datasets.

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  24. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    Since serialization means persisting object state and actual state is
    fields. So there are some dualism and both approaches has logical base
    20.08.2015 12:04 пользователь "Lars Aaberg" notifications@github.com
    написал:

    Such a strategy should be optional and should not be enabled by default.
    it will be counter intuitive compared to ordinary java-standards. But I
    agree, that such a strategy could be nice to have, when you need a little
    extra performance for large datasets.

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  25. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    So saving field means "Save object snapshot" and using properties means
    "Save recepie how to create same object". There are not silver bullet
    solution - it depends...
    20.08.2015 12:04 пользователь "Lars Aaberg" notifications@github.com
    написал:

    Such a strategy should be optional and should not be enabled by default.
    it will be counter intuitive compared to ordinary java-standards. But I
    agree, that such a strategy could be nice to have, when you need a little
    extra performance for large datasets.

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  26. aaberg commented on Aug 20, 2015

    @aaberg
    aaberg
    on Aug 20, 2015
    Owner

    Yes, it really depends on the use case. I think the best solution would be to have both strategies available, and to keep the properties-first strategy as default.

  27. aconte76 commented on Aug 20, 2015

    @aconte76
    aconte76
    on Aug 20, 2015

    For Serialization Strategy coul be useful to investigate also KRYO (faster that I know)
    https://code.google.com/p/kryo/

  28. dimzon commented on Aug 20, 2015

    @dimzon
    dimzon
    on Aug 20, 2015
    ContributorAuthor

    As I said before bottleneck is additional virtual methods into call stack
    when setters is invoked. Anyway this overhead is negligent for real life
    applications. For example with a million rows - in real life you fetch them
    for some complex processing (otherwise better to process via sql). I
    believe your processing take muuuuuuuch more time than whole fetch
    overhead.
    At over side for best perfomance we need have ability to pass lambda so
    anyone can provide handwritten optimized code to read object from
    resultset. Where and when he really need it.
    20.08.2015 12:32 пользователь "Antonio Conte" notifications@github.com
    написал:

    For Serialization Strategy coul be useful to investigate also KRYO (faster
    that I know)
    https://code.google.com/p/kryo/

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  29. dalexander01 commented on Aug 20, 2015

    @dalexander01
    dalexander01
    on Aug 20, 2015

    @dimzon GSON makes use of sun.misc.unsafe to handle serialization/deserialization using private fields. It's definitely a nice feature to have as your model objects do not require setters (thus making them mutable everywhere), but I would be worried about sun.misc.unsafe being removed in future java versions.

  30. dimzon commented on Aug 21, 2015

    @dimzon
    dimzon
    on Aug 21, 2015
    ContributorAuthor

    It's impossible to completely take away such mechanism since Java RMI
    serialization is based on it. It's only possible to rename it..
    21.08.2015 0:39 пользователь "dalexander01" notifications@github.com
    написал:

    @dimzon https://github.com/dimzon GSON makes use of sun.misc.unsafe to
    handle serialization/deserialization using private fields. It's definitely
    a nice feature to have as your model objects do not require setters (thus
    making them mutable everywhere), but I would be worried about
    sun.misc.unsafe being removed in future java versions.

    —
    Reply to this email directly or view it on GitHub
    #83 (comment).

  31. aaberg
    closed this as not plannedon Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions