poxu / runmpv

Runner, which allows to have only one mpv instance on Windows and Linux
6 stars 0 forks source link

Remove code duplication in code for reading config #36

Open poxu opened 2 years ago

poxu commented 2 years ago

runmpv can read configuration from 3 sources.

  1. Command line switches
  2. runmpv.properties
  3. Default configuration

There are settings, which have to be resolved before reading runmpv.propertes, because they are used to find runmpv.properties and to build configuration to read other settings.

executableDir

This one is used to tell runmpv where runmpv executable resides. This is where runmpv look for runmpv.properties by default. In most cases this is indeed a directory, containing runmpv executable. But if runmpv is launched from IDE for debugging, for example it should be specified manually, because then default will be target directory of maven project. And runmpv.config is not there. Also it's convenient to manually specify executable dir to point at the directory where runmpv real configuration is. To debug with actual read configuration.

Because runmpv should know excutableDir before configuration is read, it has to be taken from SotfSettings with executableDir setting name specified as String. I'd like to avoid that. Also, after config is read, RunMpvProperties#executableDir method is used to find executableDir . So there's duplicate code in runmpv initialization and RunMpvProperties .

videoDir

This is a setting which points at a directory, where currently opened video resides. Should be known before configuration is read, because it might be used to resolve %v placeholder in path settings.

Because runmpv should know videoDir before configuration is read, it has to be taken from SotfSettings with videoDir setting name specified as String. I'd like to avoid that. Also, after config is read, RunMpvProperties#video method is used to find videoDir . So there's duplicate code in runmpv initialization and RunMpvProperties .

Logging property file name

There's no setting for it yet, but it's only reasonable to assume it might be need to override default logging.properties.

User home directory

It is currently taken from System.getProperty("user.home") , but it might be useful to give user means to override that

Conclusion

With all of that said, I think a special RunMpvProperties like class but for those four settings only would make code easier to write and understand. New class will read properties from SoftSettings and I will pass CommandLineSettings, composed with defaults as argument. And if RunMpvPropertiesFromSettings uses this new class as a member (composition) then code duplication will magically disappear