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

Minor changes to improve clarity and reduce amount of code #2737

Closed
wants to merge 5 commits into from
Closed

Minor changes to improve clarity and reduce amount of code #2737

wants to merge 5 commits into from

Conversation

jjatie
Copy link
Collaborator

@jjatie jjatie commented Aug 20, 2017

Making use of defer where appropriate (e.g. CGContext().saveGState() / CGContext().restoreGState())
Using replaced if where really meaning guard, and if case with switch where appropriate. Much less nesting as a result.
Making use of Swift.min and Swift.max
Changed some Double to CGFloat where it makes sense to. This also reduces typecasting.
Refined for loops, in some cases replacing them with map, filter, and/or reduce

jacobchristie added 2 commits August 20, 2017 10:51
Making use of `defer` where appropriate (e.g. `CGContext().saveGState()` / `CGContext().restoreGState()`)
Using replaced `if` where really meaning `guard`, and `if case` with `switch` where appropriate. Much less nesting as a result.
Making use of `Swift.min` and `Swift.max`
Changed some `Double` to `CGFloat` where it makes sense to. This also reduces typecasting.
Refined `for` loops, in some cases replacing them with `map`, `filter`, and/or `reduce`
@jjatie
Copy link
Collaborator Author

jjatie commented Aug 21, 2017

There appears to be an inconsistent compiler bug preventing this from building. I am investigating further this week.

@pmairoldi
Copy link
Collaborator

Thanks for the improvements but I think we should wait until after swift 4 support is merged to do this.

@jjatie jjatie mentioned this pull request Sep 10, 2017
@jjatie
Copy link
Collaborator Author

jjatie commented Sep 18, 2017

Tests are passing fine for me locally, any thoughts on why they're failing in Travis CI?

@jjatie
Copy link
Collaborator Author

jjatie commented Nov 6, 2017

I'm going to take a more structured approach to this.

@jjatie jjatie closed this Nov 6, 2017
@jjatie jjatie deleted the clarity_refactor branch November 6, 2017 21:36
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