<Description> : Adding random for CGFloat and tests for the same. #262
Conversation
Generated by 🚫 danger |
Current coverage is 52.56% (diff: 100%)@@ master #262 diff @@
==========================================
Files 45 67 +22
Lines 2010 2966 +956
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 613 1559 +946
- Misses 1397 1407 +10
Partials 0 0
|
|
I added the change. I have no idea why NSURL tests are failing. Perhaps create an issue to look into it? |
|
@Khalian I think we need a better name for |
|
@piv199 what do you think? |
|
@vsouza says that a good name would be |
|
|
||
|
|
||
| /// EZSE: Returns a random floating point number between 0.0 and 1.0, inclusive. | ||
| public static func random() -> CGFloat { |
There was a problem hiding this comment.
@lfarah suggests better name
As for me, I would stick with random because it's the default behavior.
There was a problem hiding this comment.
We should not change working experience for already presented function unless it was wrong. Image thousand of people will get an update and crashes in their programs due to this change.
There was a problem hiding this comment.
@piv199 What if we deprecate this one and create a new? We already did this on some other methods:
/// EZSE: Checks if array contains at least 1 instance of the given object type
@available(*, deprecated: 1.8, renamed: "containsType(of:)")
public func containsInstanceOf<T>(_ element: T) -> Bool {
return containsType(of: element)
}
/// EZSE: Gets the object at the specified index, if it exists.
@available(*, deprecated: 1.8, renamed: "get(at:)")
public func get(_ index: Int) -> Element? {
return get(at: index)
}
/// EZSE: Checks if all elements in the array are true of false
@available(*, deprecated: 1.8, renamed: "testAll(is:)")
public func testIfAllIs(_ condition: Bool) -> Bool {
return testAll(is: condition)
}
|
Well I wanted to model the random around this : https://docs.oracle.com/javase/7/docs/api/java/lang/Math.html#random() But i dont mind changing it to randomFraction(). Once we reach consensus on the name I will make the change. |
| } | ||
|
|
||
| /// EZSE: Returns a random floating point number in the range min...max, inclusive. | ||
| public static func random(range: ClosedRange<CGFloat>) -> CGFloat { |
There was a problem hiding this comment.
random(in range:ClosedRange)
random(in range:Range)
what do you think?
There was a problem hiding this comment.
Hmmm I think these functions are good but random by itself is still useful.
There was a problem hiding this comment.
@lfarah yes of course, I just point to
CGFloat.random(in: 300..<600.34) instead of CGFloat.random(range: 300..<600.34)
And possibly add range if it not works, can't test. 300..<600.34 will work?
There was a problem hiding this comment.
So we have a consensus on this ? randomFraction with in: instead of range: ?
There was a problem hiding this comment.
The matter is that what range will random() generate values, if high random, then dots precision is small.
There was a problem hiding this comment.
What actually the case for random to use. Where?
There was a problem hiding this comment.
Ok I can drop the random with range then.
There was a problem hiding this comment.
@Khalian I'm still sure it would be great to have such extension, let it be 👍
There was a problem hiding this comment.
Ok so what do you want me to do at this point?
There was a problem hiding this comment.
@Khalian Personally I, nothing :)
|
Closing this pull request as reviewers unable to reach a consensus. |
|
@Khalian wanna open an issue and we can discuss and vote? |
| } | ||
|
|
||
| /// EZSE: Returns a random floating point number in the range min...max, inclusive. | ||
| public static func random(range: ClosedRange<CGFloat>) -> CGFloat { |
There was a problem hiding this comment.
random(in range: ClosedRange<CGFloat>)
| } | ||
|
|
||
| func testRandomWithinRange() { | ||
| let range = CGFloat(0.0)...CGFloat(10.0) |
There was a problem hiding this comment.
Please, test also CGFloat(0.0)..<CGFloat(10.0)
There was a problem hiding this comment.
Done. In is a keyword so used within.
<Type> : feature/test
|
@Khalian Thanks very much for your patience and contributing 👍 |
|
Yes @Khalian, you're awesome! 🎉 |
: feature/test