Uploaded image for project: 'Couchbase Java Client'
  1. Couchbase Java Client
  2. JCBC-228

a no-args constructor and an init method are needed

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.1.0, 1.1.1
    • Fix Version/s: 2.0.0
    • Component/s: Core
    • Security Level: Public
    • Labels:
      None

      Description

      Currently, the CouchbaseClient does a number of things that are not so pretty, like spinning up a thread from it's ctor. It would be better to add an additional, optional no-args constructor which expects an init method to be called with the other params needed, start thread, etc.

        Activity

        Hide
        daschl Michael Nitschinger added a comment -

        Assigning towards 1.1.2 but we can defer it to 1.1.3 as well.

        Show
        daschl Michael Nitschinger added a comment - Assigning towards 1.1.2 but we can defer it to 1.1.3 as well.
        Hide
        daschl Michael Nitschinger added a comment -

        We'll have something like this in 2.0

        Show
        daschl Michael Nitschinger added a comment - We'll have something like this in 2.0
        Hide
        ingenthr Matt Ingenthron added a comment -

        Sergey, I think this is true now in 2.0, can you verify and close this if so?

        Show
        ingenthr Matt Ingenthron added a comment - Sergey, I think this is true now in 2.0, can you verify and close this if so?
        Hide
        avsej Sergey Avseyev added a comment -

        Yes, we have static method create() which does not have any arguments, but it is just provides defaults to full constructor, which does blocking connection anyway.

        We might extract connect() method to the public interface, and modify static constructors somehow they will allow to do at least asynchronous connection. I did some sketch here (it just extracts connect() method, but still call it from constructor) http://review.couchbase.org/41397

        The patch is not meant to merged, just demonstrate my question.

        Matt, could you expand a little bit what we need to achieve?

        Does it spawn thread in constructor? No
        Does it do blocking connection? Yes

        Show
        avsej Sergey Avseyev added a comment - Yes, we have static method create() which does not have any arguments, but it is just provides defaults to full constructor, which does blocking connection anyway. We might extract connect() method to the public interface, and modify static constructors somehow they will allow to do at least asynchronous connection. I did some sketch here (it just extracts connect() method, but still call it from constructor) http://review.couchbase.org/41397 The patch is not meant to merged, just demonstrate my question. Matt, could you expand a little bit what we need to achieve? Does it spawn thread in constructor? No Does it do blocking connection? Yes
        Hide
        ingenthr Matt Ingenthron added a comment -

        Assigning this to Michael as he's the architecture owner here.

        The reason I filed this way back at -228 was that in general, a recommendation with Java is to not do anything requiring IO or threads in the ctor and separate out the actions of setting things up in a separate method. This is often helpful in some frameworks with DI, etc. Imagine for a moment that you throw an IOException from the ctor(). Chances are whatever created that object inline isn't ready to handle that exception right there. However, calling .init() on it and seeing an explicit throws IOException gives the app developer a cleaner way of abstracting the set up of the things and the initializing of the things.

        http://www.javapractices.com/topic/TopicAction.do?Id=254

        All of these things change over time though, so I trust Michael to pick a solution he can live with.

        Show
        ingenthr Matt Ingenthron added a comment - Assigning this to Michael as he's the architecture owner here. The reason I filed this way back at -228 was that in general, a recommendation with Java is to not do anything requiring IO or threads in the ctor and separate out the actions of setting things up in a separate method. This is often helpful in some frameworks with DI, etc. Imagine for a moment that you throw an IOException from the ctor(). Chances are whatever created that object inline isn't ready to handle that exception right there. However, calling .init() on it and seeing an explicit throws IOException gives the app developer a cleaner way of abstracting the set up of the things and the initializing of the things. http://www.javapractices.com/topic/TopicAction.do?Id=254 All of these things change over time though, so I trust Michael to pick a solution he can live with.
        Hide
        avsej Sergey Avseyev added a comment -

        Michael, I also think that current implementation is good. Abandoned my patch. Please resolve the ticket, or reassign back to me

        Show
        avsej Sergey Avseyev added a comment - Michael, I also think that current implementation is good. Abandoned my patch. Please resolve the ticket, or reassign back to me
        Hide
        daschl Michael Nitschinger added a comment -

        Yes, we have static factory methods now and should be good on that.

        This was actually a relict out of changing the 1.* SDK when we didn't know that 2.0 was a breaking change. So I'm going to close this out.

        Show
        daschl Michael Nitschinger added a comment - Yes, we have static factory methods now and should be good on that. This was actually a relict out of changing the 1.* SDK when we didn't know that 2.0 was a breaking change. So I'm going to close this out.
        Hide
        daschl Michael Nitschinger added a comment -

        Requirements have changed since 2.0 is a complete rewrite its not needed anymore

        Show
        daschl Michael Nitschinger added a comment - Requirements have changed since 2.0 is a complete rewrite its not needed anymore

          People

          • Assignee:
            daschl Michael Nitschinger
            Reporter:
            ingenthr Matt Ingenthron
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Gerrit Reviews

              There are no open Gerrit changes