See inline 2014-12-29 17:42 GMT+01:00 Romain Manni-Bucau <[email protected]>:
> inline as well > > 2014-12-29 17:21 GMT+01:00 Anatole Tresch <[email protected]>: > > Hi Romain/all > > > > see inline > > > > 2014-12-29 16:58 GMT+01:00 Romain Manni-Bucau <[email protected]>: > > > >> Hello Anatole, > >> > >> Tried to put all my thoughts regading current api modules: > >> > >> - Configuration: > >> > >> * looking it I feel like > >> org.apache.tamaya.Configuration#get(java.lang.String) should be the > >> only method of Configuration. I'm not sure about current(), still > >> think it shouldnt be here but in something like in > >> ConfigurationFactory but not in configuration itself - at least to be > >> aligned on common practises > > > > > > -1 for String only, type safe access is one of the most wanted feature in > > the community. > > -1 for putting it elsewhere: It is JDK 8 style to do so. On top it would > > require adding an additional artifact to the API, which is not necessary. > > Keepo the API as small as possible. If seee further use cases that > require > > additional artifacts, we may reconsider this, but as of now I would let > be > > where it is. > > > > excepted it is not consistent this way: why providing some specific > method but not "mine". Why allowing to get implicit conversion and to > provide a PropertyAdapter as parameter. > There is a clear rule set: 1) Convenience and guaranteed support (can be tested by a TCK easily) is added for the JDK wrapper types. Nothing else. 2) Normally you use the implicit type support. 3) There might be cases, where you want to support it explicitly: a) you want to provide your adapter yourself, e.g. you provide your secret PKI decryption engine, to decrypt a encrpyted password. You don't want this to register as a global adapter! b) similarly the same applies to specific application types, e.g. legacy types you don't want to be supported OOTB. c) you want to override the defaults, because you e.g. once more legacy entries. And you dont want to add the additional parser, so it is available to all other users of your system. > etc... > > I agree it is an important and nice feature but it is not a core one. > There is a separation: the internals is strictly based on String, String maps (see PropertySource). Configuration provides typed access (its more some kind of service), really all users I have talked with, want. So IMO it is one of the real core features. Removing it IMO would be a big mistake, nobody would understand that. > One thing you forgot is: as a user you want *your* logic which can be > spring, xbean, a custom one.... That's why i strongly think it should > be outside the core to ease integration with other frameworks and not > collide on conversion process. > They still can do that, simply use the String methods. But the other that do not work with such fancy frameworks, will not want to do the conversion themselves. We are building a complete API here, not only an implementtion of eg Spring's PropertySource... > > About current(): all EE stack does it otherwise ;). The fact JDK 8 > allows to mix layers doesnt mean we should do it and that's what does > current for me: mixing service and data layers. > This will change. EE is still on Java 7, so obviously this is not possible at all. Given minizing your API fottprint, especially the internals it makes more than sense. Image how java.text would look, if you build it today. Cou could simply say NumberFormat.getInstance(). You would probably even not require to expose DecimalFormat... > > > > * All "adapt-friendly" methods should go elsewhere IMO but not the > >> core API - I guess it could be a mecanism used by all libs we'll add > >> on top of it but I wouldn't bind tamaya-core-api to it since today > >> when you integrate a config with an existing factory you already have > >> it and it just adds noise the core ATM - once again I dont say to drop > >> it but just to move it to another module for now. > >> > > -1 see above (one of the most wanted features). Additionally getXXX for > > JDK wrapper types is a quite common API style in SE. Perhaps Werner can > > correct me, if I am wrong here. Definitively it helps users, since they > > now, that for these types the have guaranteed adapter support and this is > > clearly visible from the API. > > > > Maybe where we don't agree is the fact I'm sure nobody excepted > framework writers will use this layer so smaller it is better it is. I > would put the easiness effort on next layer and not this one. > > I do not agree at all. In Credit Suisse for example more or less every developer is dealing with configuration somehow. It is by far not only the framework developer! > > > >> - PropertySource: > >> > >> * not sure I get name need, for a future gui? > >> > > Yes and no, Name allows traceability and enables dynamic access of the > > configuration components from outside, e.g. from managment functionality. > > Name could be the file name and additional info. > > > > using toString can be enough while we don't need this feature (why > introducing an API if the framework does nothing with it?). > getName() has another contract than toString(). Perhaps we should clarify that. And toString() always is implemented, so people can "forget" to do so. GetName() is part of the contract they have to provide... > > > >> * think it misses something between get(String) and getProperties() - > >> == I agree with the TODO to replace getProperties() by > >> getPropertyKeys(). Wonder if we should get PropertySource and > >> ListablePropertySource instead of isScannable(). Last one would > >> implement Iterable<String> (for getPropertyKeys()). wdyt? > >> > > I would prefer as well adding the keySet() method (it was part of my > > original proposal). > > -1 for Iterable<String>: I can do that easily from the key set, and I > don't > > want to have references on my property source through whatever loops at > > runtime (possible risk of side effects). > > -1 for ListPropertySource. Most of the property sources, even remote > ones, > > will be scannable. Using instanceof to check, if an instance is scannable > > or not, seems for me to much efforts, compared to having the small > boolean > > method. > > > > Was not the point. Point was on implementer side API is easier to use. > Impl is a detail then. > Maybe. I see ot otherwise, it requires you to implement Iterable on every config source... Let's see if other opinions are there... -- *Anatole Tresch* Java Engineer & Architect, JSR Spec Lead Glärnischweg 10 CH - 8620 Wetzikon *Switzerland, Europe Zurich, GMT+1* *Twitter: @atsticks* *Blogs: **http://javaremarkables.blogspot.ch/ <http://javaremarkables.blogspot.ch/>* *Google: atsticksMobile +41-76 344 62 79*
