Introduce typed id value classes - #229
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
4cf3f83 to
941abc6
Compare
| val messageId = data.messageId?.takeIf { it.isNotBlank() } | ||
| val conversationId = data.conversationId?.takeIf { it.isNotBlank() } | ||
|
|
||
| if (messageId == null || conversationId == null) { |
| internal const val PARTICIPANT_ID_1 = "p1" | ||
| internal const val PARTICIPANT_ID_2 = "p2" | ||
| internal const val PARTICIPANT_ID_3 = "p3" | ||
| internal val PARTICIPANT_ID_1 = ParticipantId("p1") |
There was a problem hiding this comment.
Should be participantId1 as it's not const anymore
There was a problem hiding this comment.
Just curious, is it this project convention?
Asking, because in Kotlin we use screaming case for immutable top level data (even if it's not const, and even if it has delegating "get"): https://github.com/JetBrains/kotlin/blob/master/libraries%2Fstdlib%2Fsrc%2Fkotlin%2Fcoroutines%2Fintrinsics%2FIntrinsics.kt#L57
Recently it was added to coding conventions as well: https://kotlinlang.org/docs/coding-conventions.html#property-names
sdsantos
left a comment
sdsantos
left a comment
There was a problem hiding this comment.
Just one small comment regarding the empty value classes.
| val messageText: String = "", | ||
| val subjectText: String = "", | ||
| val selfParticipantId: String = "", | ||
| val selfParticipantId: ParticipantId = ParticipantId(""), |
There was a problem hiding this comment.
Does it make sense if these ParticipantId("") would be a constant to add some semantic meaning? Like ParticipantId.NO_ID or ParticipantId.UNDEFINED?
There was a problem hiding this comment.
I decided to get rid of all the ConversationId("") that aren't made for testing. I'd like to avoid creating ParticipantId.UNDEFINED, as that could confuse the logic.
There was a problem hiding this comment.
To me empty value looks weird as well; Maybe val selfParticipantId: ParticipantId? = null then?
There was a problem hiding this comment.
Now all the empty value classes are only in the tests.
941abc6 to
438a8c7
Compare
b1f7fe4 to
f0cbf8b
Compare
f0cbf8b to
846a8d8
Compare
4 participants
Closes #219
Adds value classes
ConversationId,MessageId,ParticipantId,SubId. Primitives stay at persistence and Java boundaries via.value/fromOrNull.Behavior changes:
ConversationMessageUiModelMapper.mapreturnsnullinstead of a blankmessageId, which previously became a duplicateLazyColumnkey. SomeisEmpty()checks becameisBlank(), since the id types only expose blank checks.