Skip to content

Conversation

@NekoShinobi
Copy link
Contributor

@NekoShinobi NekoShinobi commented Jan 10, 2019

Fixes Bug #210 by not crashing, and spits out a message "Not a utf-8 string"

Took the liberty to generally refactor protocols.py in two ways.

  1. I noticed one for loop that used the for key, value in dict.items():, which I then applied to all for loops where applicable.

  2. dict['key'] if key in dict else None pattern has been changed to dict.get('key') and if None is something else like 0 then you will see dict.get('key', 0)

Edit: Would also like to mention that this should improve performance slightly since it'll only do a lookup in the dict once instead of twice.

I also changed handleMessages to use a dict instead of a series of if/elif/else. It takes a more EAFP approach and makes it a little easier to read in my opinion. Let me know if you like or dislike the change.

Tested it working in a Python 3.6 environment (server + 2 clients)

Daniel Ahn and others added 2 commits January 9, 2019 21:45
…t(), handleMessages if/elif statement uses a dictionary, json.loads(line) is no longer with a bare except
@Et0h
Copy link
Contributor

Et0h commented Jan 16, 2019

I would need to do extensive testing before allowing a refactor because the last time I allowed one it broke things in subtle ways that were hard to detect and track down. I do not currently have time for this at present so I will leave this PR open for now.

Et0h added a commit that referenced this pull request Feb 1, 2019
@Et0h
Copy link
Contributor

Et0h commented Feb 1, 2019

Hopefully resolved now by fb9b3ce.

@Et0h Et0h closed this Feb 1, 2019
albertosottile pushed a commit to albertosottile/syncplay that referenced this pull request Feb 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants