Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Serialization: Make serialization methods more concise #390

Conversation

Glutexo
Copy link
Collaborator

@Glutexo Glutexo commented Aug 6, 2019

Extracted a part of #328, so the changes are more atomic and easier to review. This is a small refactoring of (de)serialization methods, so they are more concise, expressive and DRY. Logic is not affected – methods input/output is not affected at all.

  • Extracted DateTime and UUID serialization. Like this it is more DRY and testable.
  • Shortened some multi-lines to be more expressive: removed unnecessary intermittent variables, converted dict item assignments to a singe-expression dict instantiation, turned for loops into dict/list comprehensions.
  • Renamed the input/output variables to they are consistent.

Added tests for the new helper methods.

@Glutexo Glutexo self-assigned this Aug 6, 2019
@Glutexo Glutexo force-pushed the refactor_serialization_methods branch from 3d5e1ad to 1211c2c Compare August 6, 2019 11:52
Reviewed the style of the (de)serialization methods. Removed redundant
or rather cryptic instructions. Leveraged comprehensions, dried a few
bits of code. Made the code more concise and expressive. Unified names
a bit.
@Glutexo Glutexo force-pushed the refactor_serialization_methods branch from 1211c2c to 660501a Compare August 6, 2019 11:55
@Glutexo Glutexo changed the title Make serialization methods more concise Serialization: Make serialization methods more concise Oct 11, 2019
@jharting jharting added the conflict needs to be resolved and rebased label Nov 1, 2019
@Glutexo Glutexo closed this Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict needs to be resolved and rebased
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants