Conversation
ali-ince
left a comment
ali-ince
left a comment
There was a problem hiding this comment.
Great clean-up, look to all the dependencies removed in the license notices 🎉
|
|
||
| import org.slf4j.LoggerFactory | ||
|
|
||
| class VersionUtil { |
There was a problem hiding this comment.
I think we could basically go for object VersionUtil here and get rid of the companion object?
| import org.apache.kafka.common.config.AbstractConfig | ||
| import org.apache.kafka.common.config.ConfigDef | ||
|
|
||
| class ConfigUtils { |
There was a problem hiding this comment.
could go with object ConfigUtils here and remove companion object?
| class ConfigUtils { | ||
| companion object { | ||
| inline fun <reified E : Enum<E>> getEnum( | ||
| configClass: Class<E>, // <-- even if it's unused, it is needed |
There was a problem hiding this comment.
if we remove this line, and add type parameters on the caller site seems to be compiling at least.
| } | ||
| } | ||
|
|
||
| class ConfigKeyBuilder private constructor(name: String, type: ConfigDef.Type) { |
There was a problem hiding this comment.
so, just as an exercise, how would it look like if we have all the var fields public which will make them properties, getting rid of the setter methods, and change the of functions to something like this:
fun of(group: String, name: String, type: ConfigDef.Type, init: ConfigKeyBuilder.() -> Unit): ConfigDef.ConfigKey {
val builder = ConfigKeyBuilder(group, name, type)
init(builder)
return builder.build()
}
this would end up with a usage like;
this.define(
ConfigKeyBuilder.of(Neo4jConfiguration.URI, ConfigDef.Type.LIST) {
displayName = "URI"
documentation = PropertiesUtil.getProperty(Neo4jConfiguration.URI)
group = CONNECTION.title
importance = Importance.HIGH
validator =
Validators.uri("neo4j", "neo4j+s", "neo4j+ssc", "bolt", "bolt+s", "bolt+ssc")
})
not sure if it's worth the effort, but feels like more kotlin-esque.
wdyt?
| type: ConfigDef.Type, | ||
| init: ConfigKeyBuilder.() -> Unit | ||
| ): ConfigDef.ConfigKey { | ||
| val builder = ConfigKeyBuilder(group, name, type) |
There was a problem hiding this comment.
I think you can have a single constructor and a single of if you declare default values
There was a problem hiding this comment.
the first one was never called -> removed
| fun of(name: String, type: ConfigDef.Type): ConfigKeyBuilder { | ||
| return ConfigKeyBuilder(name, type) | ||
| fun of( | ||
| group: String, |
There was a problem hiding this comment.
shouldn't it be name here? (based on the corresponding constructor's signature)
There was a problem hiding this comment.
Thanks for spotting this.
meistermeier
force-pushed
the
connect_utils_removal
branch
from
9d529c8 to
1923038
Compare
meistermeier
force-pushed
the
connect_utils_removal
branch
from
1923038 to
d947d30
Compare
4 participants
No description provided.