Implement UUIDGen capability trait. #1772
Conversation
|
Make sure to run |
|
I guess you're going to have to run |
Yeah, something actually went wrong with my git, so it was left in some weird half-state :< |
| } | ||
|
|
||
| object UUIDGen { | ||
| def randomUUID[F[_]: Sync]: F[UUID] = Sync[F].delay(UUID.randomUUID()) |
There was a problem hiding this comment.
These 2 methods should require F[_] : UUIDGen bound.
And the companion object should have:
implicit def fromSync[F[_] : Sync]: UUIDGen[F] = ???There was a problem hiding this comment.
Hey, is that what You meant ? I think I didn't fully understand what You meant .
| def randomUUID[F[_]: Sync]: F[UUID] = Sync[F].delay(UUID.randomUUID()) | ||
| def fromString[F[_]: Sync](uuidString: String): F[UUID] = | ||
| Sync[F].delay(UUID.fromString(uuidString)) | ||
| } |
There was a problem hiding this comment.
Can we also add this to the companion?
def randomString[F[_]: Functor: UUIDGen]: F[String] = randomUUID[F].map(_.toString)There was a problem hiding this comment.
Sure, sounds reasonable.
| * @param uuidString a standard String UUID representation | ||
| * @return A UUID with specified value | ||
| */ | ||
| def fromString(uuidString: String): F[UUID] |
There was a problem hiding this comment.
This trait is a UUID generator. Does this method reflect the purpose of the trait? And if so, why is there no method for making a UUID from a byte array?
There was a problem hiding this comment.
So maybe we should rename the trait and add some methods?
I also don't like the Gen suffix because it's used in kernel (yet, as a prefix) for generic type classes like GenConcurrent
There was a problem hiding this comment.
Yeah, I was kinda basing on https://github.com/davenverse/fuuid here. I had an idea to implement the byte array version too, but decided to skip it for now.
IMHO, if we are doing this it should cover all basic use-cases so I think including at least a fromString would be a good idea, but happy to discuss this :)
There was a problem hiding this comment.
I tend to think this is a bit out of scope. At the very least, I would have expected it to return Option[UUID].
There was a problem hiding this comment.
Ok, will drop it and stick to random and randomString
| * @param uuidString a standard String UUID representation | ||
| * @return A UUID with specified value | ||
| */ | ||
| def fromString(uuidString: String): F[UUID] |
There was a problem hiding this comment.
I tend to think this is a bit out of scope. At the very least, I would have expected it to return Option[UUID].
| * Generates a UUID in a pseudorandom manner and returns String representation of this UUID. | ||
| * @return randomly generated UUID string | ||
| */ | ||
| def randomString: F[String] |
There was a problem hiding this comment.
I meant that the trait would have only def randomUUID: F[UUID].
While randomString would be a method on the object.
Like:
object UUIDGen {
def apply[F[_]](implicit ev: UUIDGen[F]): UUIDGen[F] = ev
implicit def fromSync[F[_]: Sync]: UUIDGen[F] = new UUIDGen[F] {
override def randomUUID: F[UUID] = Sync[F].delay(UUID.randomUUID())
}
def randomUUID[F[_]: UUIDGen]: F[UUID] = UUIDGen[F].randomUUID
def randomString[F[_]: UUIDGen: Functor]: F[String] = randomUUID[F].map(_.toString)
}Or we can just forget about it at all :)
|
Where does this one stand? |
This PR implements UUIDGen capability trait as discussed in #1741.
The implementation is as simple as possible, not sure if we need any tests for that, but if so I will be happy to add that.
The issue mentions introducing potential literal macro, IMHO it's not a very commonly used feature, so I've just decided to skip it but If we want to have it anyway, again happy to add.