Skip to content

Commit

Permalink
fix #443
Browse files Browse the repository at this point in the history 
gotta love it when a bug fix is just deleting stuff
  • Loading branch information
@JeffBezanson
JeffBezanson committed Feb 22, 2012
1 parent 37b3ccb commit 3458255
Showing 1 changed file with 0 additions and 6 deletions.
6 changes: 0 additions & 6 deletions j/range.j
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ ref(r::Range1, s::Range1{Int}) =
show(r::Range) = print(r.start,':',r.step,':',last(r))
show(r::Range1) = print(r.start,':',last(r))

start{T<:Integer}(r::Ranges{T}) = r.start
next{T<:Integer}(r::Range{T}, i) = (i, convert(T,i+step(r)))
next{T<:Integer}(r::Range1{T}, i) = (i, convert(T,i+1))
done{T<:Integer}(r::Range{T}, i) = (r.start + r.len*r.step <= i)
done{T<:Integer}(r::Range1{T}, i) = (r.start + r.len <= i)

start(r::Ranges) = 0
next(r::Range, i) = (r.start + oftype(r.start,i)*step(r), i+1)
next(r::Range1, i) = (r.start + oftype(r.start,i), i+1)
Expand Down

3 comments on commit 3458255

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

This is obviously correct. My worry would be performance. The entire reason for the integer-specific range iteration was speed. If the performance of this is fine, then we can also probably merge the two next methods.

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

I ran all the tests and perf stuff and it seems to be identical. See what you get.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

Do we still handle integer ranges in for loops as a special case?

Please sign in to comment.