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

Large refactor, Code Simplification #59

Merged
merged 6 commits into from
Feb 9, 2017
Merged

Large refactor, Code Simplification #59

merged 6 commits into from
Feb 9, 2017

Conversation

james7132
Copy link

@james7132 james7132 commented Feb 9, 2017

  • Added simplified methods to reduce boilerplate request code on WebAgent
  • Removed WebAgent function parameters where Reddit instances are already being passed.
  • Removed public setters on a most entities that shouldn't
  • Changed deserialization to use JsonSerializer.PopulateObject instead of JsonConvert.PopulateObject. Should have positive performance benefits due to avoiding dumping to strings.
  • Use more more modern C# syntax to simplify code.
  • Switch to using LINQ in areas where it would make code more readable.
  • Removed project.lock.json, since it's autogenerated.
  • Moved duplicated code within Init/InitAsync functions into constructors.
  • Added ConfigureAwait to all awaited functions.

james7132 added 6 commits February 8, 2017 22:12
* Remove extra WebAgent parameters where Reddit is already present.
* Cleanup duplicated JSON deserialization code
* Removed Init/InitAsync, moved logic to constructors
@CrustyJew
Copy link
Owner

This is awesome! Hadn't considered the public setters issue, but you are totally right, they shouldn't have been on half the stuff that was there. Really like the simplified constructors and everything.

This does tie in to some of the discussion #40 so some of the changes to LINQ may be a bit irrelevant, but could just get moved to a method body instead as it's still "cleaner" in my opinion.

Merging this in and I'll push another pre-release to NuGet. I'd like to address a couple of the discussions (#40, #42) then we can probably release a Beta for 2.0.0 I've been using the pre-release version in some bot code without issues so far, so I'm excited to see how these changes merge in.

Closes #41

@CrustyJew CrustyJew merged commit 7151108 into CrustyJew:2.0-.Net-Core Feb 9, 2017
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