Skip to main content
Code Review

Return to Answer

added 36 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval (and the existing clamp() method) might be an alternative.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval might be an alternative.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval (and the existing clamp() method) might be an alternative.

added 1 character in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

There is a general problem: The distanceTo(_: Self) -> Self.Distance of the ForwardIndexType protocol requires that

  • start and end are part of the same sequence when conforming to RandomAccessSequenceType.

  • end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be RandomAccessIndexType).

Here is a simple example where this conditions are not met, and your code crashes:

let r1 = "fπŸ‡©πŸ‡°oo".rangeOfString("πŸ‡©πŸ‡°")!
print(r1) // 1..<5
let r2 = "baπŸ‡¨πŸ‡°r".rangeOfString("πŸ‡¨πŸ‡°")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 does never hithits the start index of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
 let value: Int
 
 func successor() -> MyIndex {
 return MyIndex(value: self.value + 1)
 }
}
func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
 return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4) never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer to the same sequence. I don't know if that can be ensureensured at compile time with a suitable restriction, I assume that it is not possible. It should at least be documented (similar to the above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case of an empty intersection it a sensible choice. Is similar to that of the clamp() method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other interval lies on the left or on the right.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval might be an alternative.

There is a general problem: The distanceTo(_: Self) -> Self.Distance of the ForwardIndexType protocol requires that

  • start and end are part of the same sequence when conforming to RandomAccessSequenceType.

  • end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be RandomAccessIndexType).

Here is a simple example where this conditions are not met, and your code crashes:

let r1 = "fπŸ‡©πŸ‡°oo".rangeOfString("πŸ‡©πŸ‡°")!
print(r1) // 1..<5
let r2 = "baπŸ‡¨πŸ‡°r".rangeOfString("πŸ‡¨πŸ‡°")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 does never hit the start index of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
 let value: Int
 
 func successor() -> MyIndex {
 return MyIndex(value: self.value + 1)
 }
}
func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
 return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4) never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer to the same sequence. I don't know if that can be ensure at compile time with a suitable restriction, I assume that it is not possible. It should at least be documented (similar to the above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case of an empty intersection it a sensible choice. Is similar to that of the clamp() method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other interval lies on the left or on the right.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval might be an alternative.

There is a general problem: The distanceTo(_: Self) -> Self.Distance of the ForwardIndexType protocol requires that

  • start and end are part of the same sequence when conforming to RandomAccessSequenceType.

  • end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be RandomAccessIndexType).

Here is a simple example where this conditions are not met, and your code crashes:

let r1 = "fπŸ‡©πŸ‡°oo".rangeOfString("πŸ‡©πŸ‡°")!
print(r1) // 1..<5
let r2 = "baπŸ‡¨πŸ‡°r".rangeOfString("πŸ‡¨πŸ‡°")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 never hits the start index of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
 let value: Int
 
 func successor() -> MyIndex {
 return MyIndex(value: self.value + 1)
 }
}
func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
 return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4) never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer to the same sequence. I don't know if that can be ensured at compile time with a suitable restriction, I assume that it is not possible. It should at least be documented (similar to the above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case of an empty intersection it a sensible choice. Is similar to that of the clamp() method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other interval lies on the left or on the right.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval might be an alternative.

added 326 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

There is a general problem: The distanceTo(_: Self) -> Self.Distance of the ForwardIndexType protocol requires that

  • start and end are part of the same sequence when conforming to RandomAccessSequenceType.

  • end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be RandomAccessIndexType).

Here is a simple example where this conditions are not met, and your code crashes:

let r1 = "fπŸ‡©πŸ‡°oo".rangeOfString("πŸ‡©πŸ‡°")!
print(r1) // 1..<5
let r2 = "baπŸ‡¨πŸ‡°r".rangeOfString("πŸ‡¨πŸ‡°")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 does never hit the start index of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
 let value: Int
 
 func successor() -> MyIndex {
 return MyIndex(value: self.value + 1)
 }
}
func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
 return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4) never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer to the same sequence. I don't know if that can be ensure at compile time with a suitable restriction, I assume that it is not possible. It should at least be documented (similar to the above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case of an empty intersection it a sensible choice. Is similar to that of the clamp()clamp() method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other interval lies on the left or on the right.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval might be an alternative.

There is a general problem: The distanceTo(_: Self) -> Self.Distance of the ForwardIndexType protocol requires that

  • start and end are part of the same sequence when conforming to RandomAccessSequenceType.

  • end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be RandomAccessIndexType).

Here is a simple example where this conditions are not met, and your code crashes:

let r1 = "fπŸ‡©πŸ‡°oo".rangeOfString("πŸ‡©πŸ‡°")!
print(r1) // 1..<5
let r2 = "baπŸ‡¨πŸ‡°r".rangeOfString("πŸ‡¨πŸ‡°")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 does never hit the start index of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
 let value: Int
 
 func successor() -> MyIndex {
 return MyIndex(value: self.value + 1)
 }
}
func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
 return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4) never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer to the same sequence. I don't know if that can be ensure at compile time with a suitable restriction, I assume that it is not possible. It should at least be documented (similar to the above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case of an empty intersection it a sensible choice. Is similar to that of the clamp() method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other interval lies on the left or on the right.

There is a general problem: The distanceTo(_: Self) -> Self.Distance of the ForwardIndexType protocol requires that

  • start and end are part of the same sequence when conforming to RandomAccessSequenceType.

  • end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be RandomAccessIndexType).

Here is a simple example where this conditions are not met, and your code crashes:

let r1 = "fπŸ‡©πŸ‡°oo".rangeOfString("πŸ‡©πŸ‡°")!
print(r1) // 1..<5
let r2 = "baπŸ‡¨πŸ‡°r".rangeOfString("πŸ‡¨πŸ‡°")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 does never hit the start index of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
 let value: Int
 
 func successor() -> MyIndex {
 return MyIndex(value: self.value + 1)
 }
}
func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
 return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4) never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer to the same sequence. I don't know if that can be ensure at compile time with a suitable restriction, I assume that it is not possible. It should at least be documented (similar to the above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case of an empty intersection it a sensible choice. Is similar to that of the clamp() method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other interval lies on the left or on the right.

If your ranges just represent intervals and do not refer to indices of sequences, then ClosedInterval might be an alternative.

Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96
Loading
default

AltStyle γ«γ‚ˆγ£γ¦ε€‰ζ›γ•γ‚ŒγŸγƒšγƒΌγ‚Έ (->γ‚ͺγƒͺγ‚ΈγƒŠγƒ«) /