Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      rename setResult() to handleSendResult(). handleXXX() would be more appropriate as SendHandler is callback handler.

        Activity

        Hide
        dannycoward added a comment -

        This one's obliquely related to (http://java.net/jira/browse/WEBSOCKET_SPEC-110), but also this has come up elsewhere.

        Currently we are using 3 classes for async sends:-

        Future<SendResult>
        SendResult get()
        SendResult get(timeout, unit)
        cancel()
        isDone()
        isCancelled()

        SendHander
        setResult(SendResult sr)

        SendResult
        getException()
        isOK()

        RemoteEndpoint
        Future<SendResult> sendXXXByFuture(message)
        void sendXXXByCompletion(message, SendHandler)

        For the information transmitted during the async operation ("did it get sent or not ?"), this is perhaps a little heavyweight. It also creates an ambiguity when using the Future<SendHandler> if the operation fails: does the exception get wrapped as the cause of the ExecutionException thrown on Future.get(), or is it set on the SendResult ?

        So here's a simpler alternative, which coveys the same information, but with less API and no ambiguity:-

        Future
        void get()
        void get(timeout, unit)
        cancel()
        isDone()
        isCancelled()

        SendHandler
        void handleSendError(Throwable t)
        void handleSendCompleted();

        RemoteEndpoint
        Future sendXXXByFuture(message)
        void sendXXXByCompletion(message, SendHandler)

        A send failure using the Future method results in the Throwable being wrapped as the cause of the ExecutionException thrown out of the Future.get() method, which is, after all, the proper use for that.

        Show
        dannycoward added a comment - This one's obliquely related to ( http://java.net/jira/browse/WEBSOCKET_SPEC-110 ), but also this has come up elsewhere. Currently we are using 3 classes for async sends:- Future<SendResult> SendResult get() SendResult get(timeout, unit) cancel() isDone() isCancelled() SendHander setResult(SendResult sr) SendResult getException() isOK() RemoteEndpoint Future<SendResult> sendXXXByFuture(message) void sendXXXByCompletion(message, SendHandler) For the information transmitted during the async operation ("did it get sent or not ?"), this is perhaps a little heavyweight. It also creates an ambiguity when using the Future<SendHandler> if the operation fails: does the exception get wrapped as the cause of the ExecutionException thrown on Future.get(), or is it set on the SendResult ? So here's a simpler alternative, which coveys the same information, but with less API and no ambiguity:- Future void get() void get(timeout, unit) cancel() isDone() isCancelled() SendHandler void handleSendError(Throwable t) void handleSendCompleted(); RemoteEndpoint Future sendXXXByFuture(message) void sendXXXByCompletion(message, SendHandler) A send failure using the Future method results in the Throwable being wrapped as the cause of the ExecutionException thrown out of the Future.get() method, which is, after all, the proper use for that.
        Hide
        dannycoward added a comment -

        awaiting further feedback on this proposal from the expert group

        Show
        dannycoward added a comment - awaiting further feedback on this proposal from the expert group
        Hide
        dannycoward added a comment -

        I have modified this in v013, with one exception to the proposal: the SendResult remains, with the SendHandler having a renamed onResult(SendResult) method. The point of this it to allow SendHandlers to be lambdas.

        Show
        dannycoward added a comment - I have modified this in v013, with one exception to the proposal: the SendResult remains, with the SendHandler having a renamed onResult(SendResult) method. The point of this it to allow SendHandlers to be lambdas.

          People

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

            Dates

            • Due:
              Created:
              Updated:
              Resolved: