Fix config.Root calculation #199
Conversation
|
Ideally we should have all HOCON inner structures immutable, because all of this bugs are coming because entities reference each other and can modify each other, making it quite messy to debug. That nice moment, when the value is changed because of you just viewed it in your debugger... But this is much more major task then fixing |
|
Since #198 turned out to be a mistake, rolled back it's changes. Now should be just fine. |
|
|
||
| // If Fallback is not set - we will set it in new copy | ||
| // If Fallback was set - just use it, but with adding new fallback values | ||
| return new Config((HoconValue) Value.Clone(null), Fallback?.WithFallback(fallback) ?? fallback); |
There was a problem hiding this comment.
Turned out to be much more elegant solution for getting new config with fallback, and without internal state mutations.
| return aggregated; | ||
| } | ||
| } | ||
| public virtual HoconValue Root { get; } |
There was a problem hiding this comment.
So this is where we will safe on allocations. Root is immutable and calculated only once, in constructor
| @@ -54,27 +66,12 @@ public Config(HoconRoot root) : base(root?.Value, root?.Substitutions ?? Enumera | |||
| /// <summary> | |||
| /// The configuration used as a secondary source. | |||
| /// </summary> | |||
| public Config Fallback { get; private set; } | |||
| public Config Fallback { get; } | |||
There was a problem hiding this comment.
Fallback should never change. It is either set, or not set for given config.
| @@ -54,27 +66,12 @@ public Config(HoconRoot root) : base(root?.Value, root?.Substitutions ?? Enumera | |||
| /// <summary> | |||
| /// The configuration used as a secondary source. | |||
| /// </summary> | |||
| public Config Fallback { get; private set; } | |||
| public Config Fallback { get; } | |||
| return aggregated; | ||
| } | ||
| } | ||
| public virtual HoconValue Root { get; } |
| /// <summary> | ||
| /// Performs aggregation of Value and all Fallbacks into single <see cref="HoconValue"/> object | ||
| /// </summary> | ||
| private HoconValue GetRootValue() |
There was a problem hiding this comment.
I'm much happier with this now that it's only run once.
Close #195
@Aaronontheweb This is based on #197 , so may be better to review/merge that one first.
The main improvement here is not only the fact that
Rootis not calculated on config creation - alsoConfiginstance now is not modified due to any private methods after is created with constructor.Before there were some private setters, that were making harder to with
Configas with immutable data structure. Now it is refactored to be fully built on constructor invocation, and we are safe to write code in this class without breaking immutability (of cause if we will not touch property setters).