https://kotlinlang.org logo
Join Slack
Powered by
for constructors one must initially reference `thi...
# announcements
a
for constructors one must initially reference 
this(...)
. However, sometimes it requires a bit of code to transform parameters to 
...
. Should I... 1) have a separate function which acts as a static constructor 2) have the constructor of the class reference a function
a
are you talking about when using multiple constructors?
When dealing with multiple constructors, instead of telescoping them, I think that many of the benefits of, well named, static factory methods still exist in KT https://blog.kotlin-academy.com/effective-java-in-kotlin-item-1-consider-static-factory-methods-instead-of-constructors-8d0d7b5814b2
d
Constructors can be used in super calls, factory functions cannot. I assume that's outside the scope of the question though.
a
That is covered by Effective Java item 16, Favor composition over inheritance, which by making every class final by default, is Kotlin's default position on this.
a
@Alan Evans how would you recommend me having multiple constructors (i.e. a string constructor which has a default base path) for this then?
Copy code
abstract class BasicConfig(private val path: Path) {

    fun reload(){
        val configuration = YamlConfiguration.loadConfiguration(path.toFile())
        process(configuration)
    }

    protected abstract fun process(config: ConfigurationSection)
}
just use a regular constructor or restructure my code?
a
I actually wouldn't recommend 
abstract
classes in any situation.
Imagine:
Copy code
interface ConfigurationSectionLoader {
    fun load(): ConfigurationSection
}

class YamlConfigurationLoader(val path: Path) : ConfigurationSectionLoader {
    override fun load(): ConfigurationSection {
        return YamlConfiguration.loadConfiguration(path.toFile())
    }
}
Copy code
class YourCustomClass(private val loader: ConfigurationSectionLoader) {

    fun reload() {
        process(loader.load())
    }

    private fun process(load: ConfigurationSection) {
        TODO("not implemented")
    }
}
Now your custom class just depends on something rather than descending from something.
ConfigurationSectionLoader
interface is optional. Depends how you want to couple these.
As for default path, that's just:``` class YamlConfigurationLoader(val path: Path = File("some/default/path.yml").toPath()) : ConfigurationSectionLoader { ```
Note how 
YourCustomClass
does not need to know the path in any situation. Where as with 
abstract
it will always need to provide that option in it's constructor.
a
alright, so I've applied this pattern to a lot of code. Think it is a good idea to separate all implementations into separate classes? If I do this it seems to get pretty messy... maybe it is better to use anonymous classes instead for very specific uses?
Copy code
object PlayerSubCommand : SimpleCommand by SubCommandBase(
        1,
        CommandAlias("player", "p"),
        PlayerTabAutocomplete,
        object : SubCommandProcessor {
            override fun on(sender: CommandSender, command: Command, alias: String, args: Array<out String>): SubCommandResult {
                val input = args[0]

                val offlinePlayer = try {
                    sp.server.getOfflinePlayer(UUID.fromString(input)) // UUID
                } catch (e: IllegalArgumentException) {
                    sp.server.getOfflinePlayer(input) // name
                }

                return SubCommandResult.Success(
                        setOf(
                                PlayerAddGroupCommand(offlinePlayer),
                                PlayerRemoveGroupCommand(offlinePlayer)
                        )
                )
            }
        }
)
where
Copy code
class SubCommandBase(
        private val argsCount: Int,
        private val nameable: Nameable,
        private val tabCompleter: TabCompleter,
        private val subCommandProcessor: SubCommandProcessor) :
        SimpleCommand,
        Nameable by nameable {
...
}
a
I just fail to see what is so complex about a Command that it can't just be an interface. 4 dependencies is a lot things like 
TabCompleter
and arg count are SRP smells to me.
would like to see the body of `SubCommandBase to see what makes it need to be an object.
d
Command handling in minecraft servers is no trivial task, from experience
I recommend considering https://github.com/Mojang/brigadier
a
Obviously if you're subject to other people's architecture choices then at their API boundary there's not much you can do. But beyond that boundary you can minimize their API impact on the cleanness of your code through good abstractions, and that's what projects like brigadier are attempting to do.
d
Well, no, I disagree. I will leave it at that though
a
a
It's getting too specific for me, I can only discuss in generalities and a big part of the problem with these snippets is you can't explain the situation to me without me learning about minecraft. If that's the case, then you need to lean on domain experts like Dico.
@Dico I was only saying a library like brigadier (again me without actually learning what minecraft is) is an abstraction layer which makes dealing with minecraft commands more easy. Do you disagree with that?
And that without using a library or writing your own abstraction layer then your code is subject to all the architecture designs of the minecraft API (but also, maybe that's OK).
d
The first one no, the second yes. Brigadier allows you to build very complex command systems. It adds a lot more features on top of a relatively simple api.
In general, libraries can indeed be designed to make something more simple to use, or more accessible by supporting multiple platforms, but that isn't always the case.
a
https://kotlinlang.slack.com/archives/C0922A726/p1548678980676400?thread_ts=1548265838.440000&amp;cid=C0922A726 
SimpleCommand
is still an interface 
SubCommandBase
is just a class which combines dependencies. I don't see how this is bad...
d
I think what you're doing is good
Mostly different approach than I've seen
a
Yeah I'm not saying it's bad, I'm just saying I don't have enough of the picture to really appreciate what you're doing or offer any alternatives.
I would say that the use of 
by
here is not required:
Copy code
object PlayerSubCommand : SimpleCommand = SubCommandBase(
Would work just as well wouldn't it?
d
I wish that were valid syntax actually
Wouldn't make sense though
Have to use val
a
ah yes,
Copy code
val playerSubCommand : SimpleCommand = SubCommandBase(
I think it's clearer what's going on there, as with 
by
you might also be overriding some of the interface and I would have to look at the bottom of the declaration to find out.
Another syntactical suggestion is to put functions at the end of the list of arguments. Now imagine a function that helps you create a `SimpleCommand`:
Copy code
var playerSubCommand: SimpleCommand = simpleSubCommand(
    1,
    CommandAlias("player", "p"),
    PlayerTabAutocomplete
) { args -> // none of the other crap, I don't think you should need anything but the command's arguments
    {
        val input = args[0]

        val offlinePlayer = try {
            sp.server.getOfflinePlayer(UUID.fromString(input)) // UUID
        } catch (e: IllegalArgumentException) {
            sp.server.getOfflinePlayer(input) // name
        }
        SubCommandResult.Success(
            setOf(
                PlayerAddGroupCommand(offlinePlayer),
                PlayerRemoveGroupCommand(offlinePlayer)
            )
        )
    }
}
That function could create a 
SubCommandBase
you see. 
SubCommandBase
should be renamed in any case though, as it is no longer a "Base" class.
Or even multiple functions for 1-n arguments
Copy code
var playerSubCommand: SimpleCommand = simple2ArgSubCommand(
    CommandAlias("player", "p"),
    PlayerTabAutocomplete
) { arg1, arg2 ->
    {
        val offlinePlayer = try {
            sp.server.getOfflinePlayer(UUID.fromString(arg1)) // UUID
        } catch (e: IllegalArgumentException) {
            sp.server.getOfflinePlayer(input) // name
        }
        SubCommandResult.Success(
            setOf(
                PlayerAddGroupCommand(offlinePlayer),
                PlayerRemoveGroupCommand(offlinePlayer)
            )
        )
    }
}
❤️ 1
d
Might be worth mentioning that 
server.getOfflinePlayer
can return players that don't exist. You should check that the condition 
it.hasPlayedBefore() || it.isOnline()
is true on the result.
That is, talking about the UUID version
a
Thanks. btw @Dico I took inspiration from the link you posted (https://github.com/Mojang/brigadier) and created my own Kotlin version. Using the builder pattern is MUCH NICER than my old layout imo. https://gist.github.com/andrewgazelka/f5721f9bbb36827cae19224eeb426d4d
👍 1
d
Very nice :) you mean you added some kotlin utilities for brigadier?
a
nope... found it easier to just remake since there wasn't a whole lot of documentation and and I'll be generating help commands and whatnot which will take me a while to figure out how to do in brigadier
plus I wanted to structure things slightly different than brigadier
kinda a necro post but btw @Dico this is what I created https://github.com/SimplyServers/SimpleCommand
d
If it works for your use case, that's great. Well done :)
a
Looks nice! Consider unit tests at some point.
2 Views
Open in Slack
PreviousNext
Powered by