Issue #90 Convert to RunCommand: CountCollectionByNameStatement + int… #104
Conversation
|
Please squash the commits when merging if you are happy to include this change request. |
…uce ListCollectionNamesStatement for this and DropAllCollectionsStatement
alexandru-slobodcicov
force-pushed
the
convert_statements_to_runCommand2
branch
from
b1ed169
to
4d1a626
Compare
…tatement and DropAllCollectionsStatement to AbstractRunCommandStatement
| @Getter | ||
| @EqualsAndHashCode(callSuper = true) | ||
| public class CountCollectionByNameStatement extends AbstractCollectionStatement | ||
| public class CountCollectionByNameStatement extends ListCollectionNamesStatement |
There was a problem hiding this comment.
I'm curious - why not prefer composition over inheritance?
There was a problem hiding this comment.
Just unified logic under AbstractRunCommandStatement as in the end they are runCommand underneath. It eliminates duplication of code and logic like toJs, run, checkErrors, etc. Yes we could think about adopting composition if you give an example of real value. For now I don't see that as an urgency. We could go via SqlGenerators used by over liquibase SqlStatements however think it brings more complexity rather than value.
| public static final String FILTER = "filter"; | ||
| public static final String CURSOR = "cursor"; | ||
| public static final String FIRST_BATCH = "firstBatch"; | ||
| public static final String NAME = "name"; | ||
| public static final String AUTHORIZED_COLLECTIONS = "authorizedCollections"; | ||
| public static final String NAME_ONLY = "nameOnly"; |
There was a problem hiding this comment.
My personal style would not extract constants for these unless there's reuse as it detracts from readability in-situ (increasing the 'code distance' which increases probability of errors). There's no performance penalty since the compiler will optimise the String literals anyway.
Could these be private static final String... too?
Noted that this is the style you would like in the project though, so will follow it moving forward.
There was a problem hiding this comment.
Yes, that's style and some of our Sonar rules related to magic values. I prefer the fields used by mongo driver to be as constants, even for the current set NAME is reused. For example Cosmos java driver is exposing constants for their fields which I'm using in the liquibase-cosmosdb extension. Related to whether the constants have to be private, I usually don't like hiding the things that do not worth hiding, if they are potentially to be used in other Statements.
Hopefully this style aspects won't irritate you and you will continue contributing :)
introduce
ListCollectionNamesStatement(as arunCommandstatement) to be used byCountCollectionByNameStatementDropAllCollectionsStatementEdited
DropCollectionStatementto support options ( as per https://docs.mongodb.com/manual/reference/command/drop/ )┆Issue is synchronized with this Jira Bug by Unito