websocket-spec
  1. websocket-spec
  2. WEBSOCKET_SPEC-85

Some DefaultClientConfiguration methods return ClientEndpointConfiguration

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      public ClientEndpointConfiguration setDecoders(List<Decoder> decoders) {

      that should be

      public DefaultClientConfiguration setDecoders(List<Decoder> decoders) {

      Similarly, other methods setEncoders, setExtensions, setPreferredSubProtocols needs to be changed

        Activity

        Hide
        Justin Lee added a comment -

        My first question was, "why would you want to return a concrete class rather than the interface" but then noticed that those methods aren't on the interface at all. Why is that? Seems like an oversight.

        Show
        Justin Lee added a comment - My first question was, "why would you want to return a concrete class rather than the interface" but then noticed that those methods aren't on the interface at all. Why is that? Seems like an oversight.
        Hide
        jitu added a comment -

        IMO, we shouldn't be adding the setter methods on the ClientEndpointConfiguration interface or its super interfaces. Container doesn't use these setter methods. Also, all the concrete impl will be forced to be mutable.

        DefaultClientConfiguration is just one concrete impl that is using these setter methods to configure that information. Instead, it can use constructors, or a builder (if there are many parameters) to configure that information.

        Show
        jitu added a comment - IMO, we shouldn't be adding the setter methods on the ClientEndpointConfiguration interface or its super interfaces. Container doesn't use these setter methods. Also, all the concrete impl will be forced to be mutable. DefaultClientConfiguration is just one concrete impl that is using these setter methods to configure that information. Instead, it can use constructors, or a builder (if there are many parameters) to configure that information.
        Hide
        dannycoward added a comment -

        This is an oversight. Its trying to use the builder pattern to set up the information. So the returns should be DefaultClientConfiguration.

        Show
        dannycoward added a comment - This is an oversight. Its trying to use the builder pattern to set up the information. So the returns should be DefaultClientConfiguration.
        Hide
        dannycoward added a comment -

        fixed as suggested.

        Show
        dannycoward added a comment - fixed as suggested.

          People

          • Assignee:
            dannycoward
            Reporter:
            jitu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved: