Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: > On Tue, Feb 14, 2017 at 3:32 PM, Daniel Dekany <[email protected]> wrote: >> I propose that in FM3 Configuration should only have two constructors: >> >> Configuration(Version incompatibleImprovements) >> Configuration(Properties properties) > > Is it to make sure that users should set the version compatibility > explicitly by removing the default constructor?
Yes. In the case of Configuration(Properties properties) my idea was that if the user sets the incompatibleImprovements in it, we can apply that first and in the constructor too. However, I realize that then it should be Configuration(Properties properties, Version defaultIncompatibleImprovements). Also, since then I start to think that the cfg.isXxxExplicitlySet() are useful even regardless of incompatibleImprovements, like they were utilized in FreemarkerServlet. Plus in Configurable (the superclass of Environment and Tempalte and Configuration) we need them to know when to fall back. And if we have the isXxxExplicitlySet-s after all, then most of the advantage of enforcing setting incompatibleImprovements first and only once is gone... > I thought it is a good option with the default constructor because > someone always want to use the latest default incompatible improvement > anyway with being willing to fix any incompatible template code. > Or could it have been dangerous sometimes? That would defeat the purpose of incompatibleImprovements. Adding a not entirely backward compatible fix would break backward compatibility, and so it couldn't be added (until and if ever something dramatic like FM4 happens). >> and that it should not have a setIncompatibleImprovements(Version) >> method. Because then it can be ensured that >> cfg.incompatibleImprovements is always set first, and not modified >> anymore. It's simplifies life considerably, because changing the >> incompatibleImprovements changes the default of some settings, so when >> in FM2 you change it at some later point, we will have to figure out >> which settings were never set with the public setter method, and thus >> still hold the initial default (not just something that's identical to >> it!), and thus has to be changed. Because Properties are esentially >> unordered, we also have to get all of them at once, so that we can >> sort them (bring incompatibleImprovements to the first place, sort any >> other dependent settings too). > > +1 > >> >> I also intend to remove `String getSetting(String)`, because it can't >> be implemented and has never worked. > > +1 > >> >> I'm also very inclined to remove setSetting(String) and >> setSettings(Properties), as it can lure the user into some ordering >> traps. I think Configuration(Properties properties) should be enough, >> and then, the user is forced to collect all the properties together >> (coming form different source maybe, that's why he collects them), and >> the we can do a proper ordering because we have all of them at once. > > +1 > >> >> Thoughts? > > I think any operations, which could affect or be affected by > invocation orders, should be removed. That will make the code cleaner > and safer. Since then I have doubts about my own somewhat impulsive proposal... Yes, taking out invocation order from the equation was a goal, but since then I realize that solving that for j.u.Properties-based configuring is not enough at all. More modern applications (frameworks) should use the setter methods (as in Spring IoC for example). But when you set JavaBean properties, do you used to care abut the order? Even if you can influence the order (though not sure what Spring does if you configure something with XML), I think usually you don't pay attention to that. So being strict about when and how j.u.Properties can be applied doesn't gets us far, as we have to be order independent with the setter anyway. > Also, if possible, I would like to have an immutable object or > representation than the Configuration itself. For example, > template#getConfiguration doesn't have to return a mutable object, for > instance. But it requires a bigger change, I guess. Yeah, I will respond to this in another thread. > Cheers, > > Woonsan > >> >> -- >> Thanks, >> Daniel Dekany >> > -- Thanks, Daniel Dekany
