Skip to content
This repository has been archived by the owner on Oct 28, 2019. It is now read-only.

Null reference caused "Badge page didn't load" #243

Merged
merged 1 commit into from
Jan 5, 2017

Conversation

treloret
Copy link
Contributor

@treloret treloret commented Jan 4, 2017

Simple null check seems to have solved it, steamLogin wasn't available

Simple null check seems to have solved it.
@treloret treloret closed this Jan 4, 2017
@treloret treloret reopened this Jan 4, 2017
@treloret
Copy link
Contributor Author

treloret commented Jan 4, 2017

Thought this was a similar to a fix that was proposed december 2015, seems I fixed it with a less brute force approach.

@opello
Copy link

opello commented Jan 6, 2017

I disagree that this approach is less brute force but since @jshackles merged it I suppose that doesn't matter. This does 2 iterations of the CookieCollection, one for each Item[String] access, while the update I made to #197 caches the Cookie to only do one lookup.

@treloret treloret deleted the patch-1 branch January 6, 2017 10:51
@treloret
Copy link
Contributor Author

treloret commented Jan 6, 2017

You're bit hung up on that 'brute force' thing, and yes yours is the optimized derivative fix from mine.

@opello
Copy link

opello commented Jan 6, 2017

I probably appear "hung up" because I wanted to address it in the various places the problem or fix was being discussed so that everyone that might care was aware of what was happening.

My original fix was more efficient, despite your criticism, with only one iteration of the CookieCollection. However, a design goal may be to conceal implementation details which I think is reasonable.

I'm not sure what you mean by derivative and it seems unnecessarily pejorative. I wouldn't have pushed it into my branch and merged the conflicts from upstream if I didn't think it was a better solution. I really just want the best solution to be employed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants