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

Code Review Suggestions #1

Open
ColinM9991-zz opened this issue Aug 21, 2017 · 0 comments
Open

Code Review Suggestions #1

ColinM9991-zz opened this issue Aug 21, 2017 · 0 comments

Comments

@ColinM9991-zz
Copy link

Suggestion #1:
HttpClient is intended to be reused, instead of creating multiple instances for a request, you can use one HttpClient instance to handle multiple asynchronous requests
Line:

HttpClient client = new HttpClient();

Suggestion #2:
The ToEvent method looks brittle, it uses long.Parse which will through various exceptions in certain cases, there are no sanity checks performed to see if the string is a correct Int64 and if it contains content. It may be good to use TryParse(string, out long);
Line:

public static Event ToEvent(this RawEvent @event)

Suggestion #3:
Instead of concatenating the URL you could use a UriBuilder to do this for you, and pass a Uri object to your GetX methods. Using a Uri object will add an extra bit of sanity that says this is a correct URL before it gets as far as attempting to send a request.
Line:

item.Venue = GetVenue(requests.VenueGetUrl + item.Venue + @"/?" + requests.AuthToken).Result;

Suggestion 4:
It isn't necessary to use an @ symbol when indexing your deserialized JObject

Suggestion 5:
It is safer to deserialize to a strong object instead of indexing on key names, spelling mistakes could happen.

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

No branches or pull requests

1 participant