Uploaded image for project: 'Kafka'
  1. Kafka
  2. KAFKA-1040

ConsumerConfig and ProducerConfig do "work" in the Constructor

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Won't Fix
    • Affects Version/s: 0.8.0
    • Fix Version/s: None
    • Component/s: config, consumer, producer
    • Labels:
    • Environment:
      Java 1.7
      Linux Mint 14 (64bit)

      Description

      It appears that validation of configuration properties is performed in the ConsumerConfig and ProducerConfig constructors. This is generally bad practice as it couples object construction and validation. It also makes it difficult to mock these objects in unit tests.

      Ideally validation of the configuration properties should be separated from object construction and initiated by those that rely/use these config objects.

      http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

        Activity

        Hide
        gwenshap Gwen Shapira added a comment -

        Closing as won't fix since I don't think anyone has a problem with the current behavior.

        Show
        gwenshap Gwen Shapira added a comment - Closing as won't fix since I don't think anyone has a problem with the current behavior.
        Hide
        jkreps Jay Kreps added a comment -

        Not sure I get the analogy, can we be concrete about the problem this solves. Is this meant to help the end user of Kafka or the Kafka developers or both? What problem do either of these two people currently have that this change would fix? You mentioned mocking, but these classes are internal and we provide a mock of the client for the end user...

        Show
        jkreps Jay Kreps added a comment - Not sure I get the analogy, can we be concrete about the problem this solves. Is this meant to help the end user of Kafka or the Kafka developers or both? What problem do either of these two people currently have that this change would fix? You mentioned mocking, but these classes are internal and we provide a mock of the client for the end user...
        Hide
        saden1 Sharmarke Aden added a comment - - edited

        Suppose you want to build your dream house and contract a builder. Who validates the house is being built to specification? You, the contractor or the house?

        Show
        saden1 Sharmarke Aden added a comment - - edited Suppose you want to build your dream house and contract a builder. Who validates the house is being built to specification? You, the contractor or the house?
        Hide
        jkreps Jay Kreps added a comment -

        Hey guys, I'm not sure I agree with this principle. The goal of a constructor is to construct a valid instance of the object. So validating the constructor arguments should totally be done in the constructor. Could you give a concrete example of how adding a separate validate() method would make things better?

        Show
        jkreps Jay Kreps added a comment - Hey guys, I'm not sure I agree with this principle. The goal of a constructor is to construct a valid instance of the object. So validating the constructor arguments should totally be done in the constructor. Could you give a concrete example of how adding a separate validate() method would make things better?

          People

          • Assignee:
            nehanarkhede Neha Narkhede
            Reporter:
            saden1 Sharmarke Aden
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development