Overall
Overall Remarks
QuestionsQuestion Responses
Suggestions
But with an arrow function the context is the same... so there is no need to assign the outer context. If it wasn't using an arrow function, the same could be achieved with Function.bind()
Function.bind()
.
Overall
Questions
But with an arrow function the context is the same... so there is no need to assign the outer context. If it wasn't using an arrow function, the same could be achieved with Function.bind()
.
Overall Remarks
Question Responses
Suggestions
But with an arrow function the context is the same... so there is no need to assign the outer context. If it wasn't using an arrow function, the same could be achieved with Function.bind()
.
Many argue that const
and let
should be used instead of var
const
and let
should be used instead of var
in ES6 (given class syntax used) for multiple reasons - e.g. block scoping, unintentional re-assignment with const
, etc.
Many argue that const
and let
should be used instead of var
in ES6 (given class syntax used) for multiple reasons - e.g. block scoping, unintentional re-assignment with const
, etc.
Many argue that const
and let
should be used instead of var
in ES6 for multiple reasons - e.g. block scoping, unintentional re-assignment with const
, etc.
Overall
I must admit that this code makes use of more bitwise operators than I normally see in JavaScript. Nonetheless it looks to be sophisticated. There is a lot of code so the remarks here may not be comprehensive but I’ll cover what I can.
Questions
- Is
Object.freeze()
the normal way to implement enum logic in JavaScript?
That seems to be the common convention. Note that "freeze is shallow"1 so if there were nested objects then it would likely by obligatory to freeze those as well. Refer to answers to this Stack Overflow question: What is the preferred syntax for defining enums in JavaScript?. For even more light reading take a glance at this article posted by one of the users who supplied an answer to that aforementioned SO question.
- Is it okay to give same names for method and static method?
While it was closed as off-topic that question was asked on Stack Overflow as well.
- Is
instance.constructor.static_method()
the normal way to access to a static method?
I don’t want to sound like a broken record but there’s a SO question for that. That was works unless the goal is to allow an override static method to be used in a sub-class.
- Is the way i use Promise correct?
I am not sure if it is "correct" but as far as I can tell that seems to match typical usage I have seen.
- Is naming style I used for classes, variables, constants and methods correct?
Again I'm not sure what "correct" is but we can say it is idiomatic - it appears to follow standard conventions (e.g. style guides like AirBnB, Google, etc.)
Variable declaration keywords
There are quite a few lines that use keyword var
.
var number = 0, bytes_read = 0;
var toWrite = number & 0x7F;
Many argue that const
and let
should be used instead of var
in ES6 (given class syntax used) for multiple reasons - e.g. block scoping, unintentional re-assignment with const
, etc.
Binding context to this
In TLSClient::send()
there is this first line:
var client = this;
that appears to be used within the promise callback - e.g.:
client.socket.write(Buffer.concat([length, encoded_packet]));
But with an arrow function the context is the same... so there is no need to assign the outer context. If it wasn't using an arrow function, the same could be achieved with Function.bind()
.
Extra lambda functions
In TLSClient::send()
there is this declaration:
client.socket.on("error", (err) => { reject(err); });
This could be simplified to:
this.socket.on("error", reject);
because the arguments would get passed directly to the callback.
Bloated for
loop
In Message::decode()
there is a loop started like this:
for (; bytes_read < length;) {
It could simple be a while
loop:
while (bytes_read < length) {