Any reason not to use a `LinkedHashMap` as before?
jasonsparc opened this issue · 3 comments
Recently, I updated to 4.0.4
and apparently I wasn't aware that the ordering of the fields are no longer being taken into account. The previous implementation used to use a LinkedHashMap
to collect the fields, and I took advantage of that fact and have my fields depend on each other.
Now sadly, the current implementation now uses a NamedDomainObjectContainer
.
As far as I know, you could use a Map
for @Nested
(according to its API docs), and thus, you should be able to use a LinkedHashMap
with it. Now I'm not sure how it would behave if the configuration cache is enabled, but let's just assume that, that would be a Gradle bug instead; since after all, the configuration cache should be able to restore objects back to their original types (or throw an error otherwise if it can't).
Not a particular reason, just felt more correct to use it. I think I didn't consider the use case of making constants depending on others.
I could use another implementation that keeps the insertion order, but I don't want to add another backward incompatible change.
I think a what I can do is to add an order
parameter that will automatically increment on each add. WDYT?
I think a what I can do is to add an
order
parameter that will automatically increment on each add. WDYT?
I think that's an acceptable compromise.
Looking at the PR #58, I think I'm fine with that – as a bonus, it's also less dependent on Gradle behavior (unlike the LinkedHashMap
implementation that I mentioned).
Maybe also add a test that does something similar to the following:
buildConfig {
buildConfigField("String", "FIRST", "1")
buildConfigField("String", "SECOND", "FIRST + 1")
buildConfigField("String", "THIRD", "SECOND + 1")
}
Asserting that FIRST == 1 && SECOND == 2 && THIRD == 3
upon generating the build config constants.
Thanks for reporting this. I've added a test and it will be released as 4.1.0