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

Fixed deprecation warnings and index increment #21

Merged
merged 5 commits into from
Apr 6, 2016

Conversation

vfn
Copy link
Member

@vfn vfn commented Mar 31, 2016

The if inside the generate methods would test index and then increment it, allowing it to go out of bounds.

@andreacremaschi
Copy link
Collaborator

Hey @vfn thank you for the PR
Oh, I see, the index thing was definitely wrong, wonder why it didn't pop out before.
I'll check why Travis is complaining before merging.

@vfn vfn closed this Apr 4, 2016
@vfn vfn reopened this Apr 4, 2016
@vfn
Copy link
Member Author

vfn commented Apr 4, 2016

@andreacremaschi I forgot to update the travis file to use xcode7.3. The PR has been updated, build and tests are fine, but Travis is now crashing https://travis-ci.org/andreacremaschi/GEOSwift/builds/120500155

It looks like a xctool issue facebookarchive/xctool#666

@@ -184,9 +184,10 @@ public struct CoordinatesCollection: SequenceType {

public func generate() -> AnyGenerator<Coordinate> {
var index: UInt32 = 0
return anyGenerator {
return AnyGenerator {
index += 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second check, this is wrong: index should be incremented AFTER accessing the element, not BEFORE, something like this:

    public func generate() -> AnyGenerator<T> {
        var index: Int32 = 0
        return AnyGenerator {
            if index < self.count {
                let item = self[index]
                index = index + 1
                return item
            }
            return nil
        }
    }

@vfn
Copy link
Member Author

vfn commented Apr 5, 2016

@andreacremaschi andreacremaschi merged commit 3ded8ce into GEOSwift:develop Apr 6, 2016
@andreacremaschi
Copy link
Collaborator

thanks! 🍻

@vfn
Copy link
Member Author

vfn commented Apr 6, 2016

👍

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