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

Fix bug in initial and rest function. Dont use array.filter function #29

Merged
merged 1 commit into from Jun 17, 2014

Conversation

kiennt
Copy link
Contributor

@kiennt kiennt commented Jun 16, 2014

Fixed bug when using range to implement initial and rest

I dont think we should use array.filter for performance issue
Let 's consider this example

let a = []
for item in 1..100 { a += item }
a.filter { println($0); return $0 % 10 == 0 }

# println function will be called 200 times

@ankurp
Copy link
Owner

ankurp commented Jun 16, 2014

Thanks let me take a look.

class func contains<T: Equatable>(array: T[], value: T) -> Bool {
return array.filter({ $0 as? T == value }).count > 0
return self.filter(array) { $0 as? T == value }.count > 0
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure what the performance issue is here? If you accept an iterator you will have the println issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let 's consider this example

let evenFilter = { (num: Int) -> Bool in
    println(num)
    return num % 2 == 0
}


let array = [1, 4, 5, 32, 12, 2]
let b = array.filter(evenFilter)
println("b = \(b)")
# 1
# 4
# 5
# 32
# 12
# 2
# 1
# 4
# 5
# 32
# 12
# 2
# b = [4, 32, 12, 2]

It seems native filter function has a bug which make closure in filter run twice.

That 's reason I implement filter by myself.

@ankurp
Copy link
Owner

ankurp commented Jun 16, 2014

The initial and rest changes are good. Let's not create a filter and use the native array filter method as the issue you mentioned is possible even in our own implementation of filter.

@ankurp
Copy link
Owner

ankurp commented Jun 16, 2014

Seems like a bug in Swift language itself. Let me find out in the Apple developer forum. If it's a bug then we should wait for it to be fixed as swift compiler is buggy and not fully implemented yet.

@ankurp
Copy link
Owner

ankurp commented Jun 17, 2014

@kiennt You will have to pull from master and rebase. For now do you want to move the filter changes to another branch before we figure out if its a bug in swift or they are intentionally looping through the array twice. I think its a bug in filter code which Apple will fix when iOS8/Mac OS X is released.

@kiennt
Copy link
Contributor Author

kiennt commented Jun 17, 2014

@ankurp Alright, let me extract filter change to another branch, and rebase for this pull request

@ankurp
Copy link
Owner

ankurp commented Jun 17, 2014

@kiennt Thanks

@ankurp
Copy link
Owner

ankurp commented Jun 17, 2014

👍

ankurp added a commit that referenced this pull request Jun 17, 2014
Fix bug in `initial` and `rest` function. Dont use `array.filter` function
@ankurp ankurp merged commit 2e3a2e0 into ankurp:master Jun 17, 2014
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.

None yet

2 participants