Add Generators For Hex Char And String Values #470
Conversation
|
Is there anything I need to do to get this merged? |
|
This seems like a nice first contribution. Making this generally useful might be a little more complicated, though. I wonder how much it is needed. What context were you using it for? |
| Gen.choose(97.toChar, 102.toChar), // a-f | ||
| Gen.choose(65.toChar, 70.toChar) // A-F | ||
| ) | ||
|
|
There was a problem hiding this comment.
This could be:
def hexChar: Gen[Char] = Gen.frequency(
10 -> Gen.oneOf('0' to '9'),
3 -> Gen.oneOf('a' to 'f'),
3 -> Gen.oneOf('A' to 'F'))There was a problem hiding this comment.
I agree. I was trying to be consistent with the rest of the file.
| */ | ||
| def hexStr: Gen[String] = | ||
| listOf(hexChar).map(_.mkString) | ||
|
|
There was a problem hiding this comment.
This produces a lengthy string by default (up to 100?). That works for your test with BigInteger, but may not for when people want byte, int or long from hex?
I guess you've copied the definition of Gen.numStr. I guess it has these same sort of problems.
There was a problem hiding this comment.
Again, I agree. I did this for consistency. That being said, I'm not too worried about this. If people want to create a String which represents a bounded type, such as i32, then it would be consistent with current idioms to just map on this generator.
|
@ashawley parsing or emitting is, in my experience, a pretty common task in CS domains. My specific use case was emitting values for an API which required hex numbers encoded in a JSON string. |
|
@ChristopherDavenport any thoughts on merging this? |
|
@isomarcte Hey, I'm going through the PRs one more time before 1.14.1 and came back to this. I'm not necessarily wedded to @ashawley's proposed definition, but I do think it would be best if we had equal probability of generating each of the 16 hex digits (right now the last 6 digits are twice as likely as the first 10). You could fix this the way @ashawley suggested, or by another method (I sort of like I think your rationale for generating large strings in I'm sure it's annoying to get this feedback after so long, but thanks for sticking with this! |
|
@non it's not annoying at all. I completely agree about your reasoning with the probability being equally distributed across the domain. I'll update it. Your proposed definition looks great to me. |
|
@non updated |
isomarcte
force-pushed
the
hex-string-gen
branch
from
55d504c to
260178b
Compare
|
Looks good! 👍 |
No description provided.