Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes protocol value checking before converting to string #360

Merged
merged 1 commit into from
Apr 30, 2016

Conversation

cantidio
Copy link
Contributor

fixes #359

@@ -378,7 +378,7 @@ function encodeMessage(message) {

var value = message.value;

if (value !== null) {
if (value !== null && value !== undefined) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be written as:

if (value != null) {

@estliberitas
Copy link
Contributor

LGTM except minor nit described by @hyperlink

@cantidio
Copy link
Contributor Author

@estliberitas @hyperlink I would agree with this, but sometimes this type of abstraction is not very good.
For instance all comparisons on the project are made using strict checking with '==='. Then we start introducing abstract checking it makes things confusing.

For instance you can open the same file in the future for a refactor and think that this '==' is a mistake and change it back to '==='. Tests could help preventing this, but that's my only concern. Linters would complain about abstract checking as well.

It's more of a question of abstract vs strict checking.

But up to you guys, I can change it if you really thing that my point doesn't make sense. :)

@hyperlink hyperlink merged commit 3813c7d into SOHU-Co:master Apr 30, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error while producing messages
3 participants