JS Mqtt - PR #6 Protocol implementation#702
JS Mqtt - PR #6 Protocol implementation#702bretambrose wants to merge 16 commits intoJsMqtt-5-Validationfrom
Conversation
sbSteveK
left a comment
sbSteveK
left a comment
There was a problem hiding this comment.
Mainly trivial but one area of concern in regard to rejoining a session operation order.
|
|
||
| // user operations require more logic since there are certain conditions that we cannot | ||
| // dequeue them (receive maximum limit, no packet ids, etc...) | ||
| let id = this.userOperationQueue.shift(); |
There was a problem hiding this comment.
Trivial: Instead of shift() unshift() maybe we should use let id = this.userOperationQueue[0]
https://stackoverflow.com/questions/42501871/peek-operation-in-stack-using-javascript
This should return undefined if nothing is there and we can check against this.canDequeueUserOperation without the need to unshift it back in place.
There was a problem hiding this comment.
I'm not comfortable using an array as a filterable dequeue (example is a strict stack) using indexing operations.
| while (!done) { | ||
| let currentOperation : ClientOperation | undefined = undefined; | ||
| if (this.currentOperation != undefined) { | ||
| currentOperation = this.operations.get(this.currentOperation); |
There was a problem hiding this comment.
It's a little confusing that this.currentOperation is the operations id ,while currentOperation is the operation object. Maybe rename it to this.currentOperationId?
3 participants
Implementation of the client-side protocol requirements for MQTT 311 and 5
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.