← Back to team overview

kicad-developers team mailing list archive

Re: Why is the advanced config object read-only?


On 29/04/2019 04:25, Jon Evans wrote:> Is there any particular philosophical or technical reason why
> ADVANCED_CONFIG was created as a read-only system (i.e. to modify any
> values, you are supposed to edit the text config file and restart the
> program)?

More of a philosophical reason.

Mostly because the ADVANCED_CONFIG object has one job, and one job only: presenting a simple (type safe) view of what advanced config is present, and providing suitable defaults if not, while hiding the implementation (a file).

It's a singleton because it's designed to be very simple to access and use. Singletons are (generally) bad news as they transgress normal expectations of concurrency, ownership and access. This is why it's const, to ensure 1) it's always consistent and 2) to avoid use as a general-purpose about:config-style read/write store for random bits and bobs (which leads to risky changes to global state that A_C is not designed to address).

It's not expected to change at runtime (if it's supposed to be changeable by the user, then it's not advanced config, it's just config, and it should have proper UI).

Co-opting advanced config for storing domain-specific state that comes from internal processes far away in the code seems a little bit mushy in terms of separation of concerns.

Doubly so in the case of things that might change over time. Then you might start running into consistency issues with multiple users of A_C getting different answers. If clients want to change their own parameters, they should implement their own class invariants, where A_C is just one of the inputs to the decision.

> (The use case is that real-time connectivity is currently an advanced
> config that defaults to off.  I would like to change it so that
> real-time is defaulted to on, but can be disabled in advanced config,
> and can also be disabled if an internal performance threshold trips.
> The easiest way to do this would be to modify the state of the
> ADVANCED_CONFIG singleton at runtime.)

I suggest having an advanced config "Disable Realtime Connectivity" as the override and combine this with the internal perf threshold at the point of use:

E.g. somewhere suitable, compute (and cache as needed) this value:

bool isRealTime = !ADVANCED_CFG::GetCfg().m_realTimeConnectivityDisabled && connectivityPerfOKForRealTime();

Then if you need to change this value, you can implement your own mechanisms for ensuring all interested code has the right value of isRealTime (mutex, signal, event, etc etc) whenever the domain-local conditions change.



Follow ups