-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#23: normalizing Scala -> PG objects #113
Conversation
JaCoCo agent module code coverage report - spark:2 - scala 2.12.12
|
JaCoCo server module code coverage report - scala 2.12.12
|
} | ||
|
||
private def scalaSeqToPgArray(toConvert: Seq[String]): String = { | ||
val scalaArrayAndItsContent: Regex = "^\\[(.*)\\]$".r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment what the Regex matches.
Also I have a suspicion, it might not be correct. 😟
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored
|
||
private def scalaSeqToPgArray(toConvert: Seq[String]): String = { | ||
val scalaArrayAndItsContent: Regex = "^\\[(.*)\\]$".r | ||
val scalaSeqJsonized = SerializationUtils.asJson(toConvert) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Jsonizing" any string? Do we really want/need that? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explained via a quick chat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe there's a generic recursive solution, but this is good and enough:
- code reviewed
- pulled
- built
- run
val scalaSeqJsonized = SerializationUtils.asJson(toConvert) // this also correctly escapes double quotes | ||
"{" + scalaSeqJsonized.substring(1, scalaSeqJsonized.length - 1) + "}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor:
val scalaSeqJsonized = SerializationUtils.asJson(toConvert) // this also correctly escapes double quotes | |
"{" + scalaSeqJsonized.substring(1, scalaSeqJsonized.length - 1) + "}" | |
val scalaSeqJsonized = SerializationUtils.asJson(toConvert) // this also correctly escapes double quotes | |
"{" + scalaSeqJsonized.substring(1, scalaSeqJsonized.length - 1) + "}" //...only the square brackets have to be replaced with curly ones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's better, thanks, I'll change it :)
Related to #23, but won't fully implement everything in that ticket. I'll split it into more PRs because #23 is dependant on DB code that is still not fully done&merged.