Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a deep copy of a configuration? #132

Open
brainbytes42 opened this issue Jan 16, 2023 · 5 comments
Open

Create a deep copy of a configuration? #132

brainbytes42 opened this issue Jan 16, 2023 · 5 comments

Comments

@brainbytes42
Copy link

hi again,

is there any (simple) way to create a deep copy of a configuration? Then one could compare the original and copied version, if one was changed later on? (equals seems to work properly for this case, comparing values and recursively including sub-configurations.)

This is slightly related to #118, as I was evaluating workarounds to this and to the question, if the config has been changed (to then manually / optionally save the config with user interaction). (Not only for autosave, but also for changes in config, a listener would be nice.)

But it seems, that in a copy only the 'simple' values are copied; sub-configurations remain linked and will be changed in both places:

Config config = Config.inMemory();
config.set("foo", "bar");
Config sub = config.createSubConfig();
sub.set("hello", "world");
config.set("sub", sub);
System.out.println("config = " + config);

System.out.println("--- COPY + CHANGE ---");

Config copy = Config.copy(config);
copy.set("foo", "bar-changed");
copy.<Config>get("sub")
    .set("hello", "world-changed");
System.out.println("config = " + config + "     <--  EXPECTED hello='world', BUT IS 'world-changed'! (no real deep copy!)");
System.out.println("copy = " + copy);

Maybe (if this behaviour isn't considered a bug), add Config.deepCopy(UnmodifiableConfig)?

@TheElectronWill
Copy link
Owner

Hi!

I've created #133 to track your feature request about modification tracking.
Config#copy() is intended to be a shallow copy, in a similar way to clone(). A deepCopy() or deepClone() would be useful, though. Let's track this here.

For now, you can implement it yourself by calling copy, iterating on the entries and recursively copying sub-configurations, it shouldn't be too long to implement.

@TheElectronWill TheElectronWill changed the title independent deep-copy and check for modification Create a deep copy of a configuration? Jan 16, 2023
@TheElectronWill TheElectronWill self-assigned this Jan 28, 2024
@TheElectronWill
Copy link
Owner

TheElectronWill commented Mar 8, 2024

I'm still wondering what the right interface is.
One possibility is to add these to Config

	/**
	 * Puts a deep copy of the given configuration to the root of this config.
	 * <p>
	 * This methods calls {@code deepCopy} on every sub-configuration,
	 * manually copies lists of known type (including {@code ArrayList}),
	 * calls {@code clone()} on {@code Cloneable} values, and simply
	 * copy the reference of other values.
	 * <p>
	 * For more control over the deep copy, in particular if you have
	 * put nested mutable objects into the config, use {@link #putDeepCopy(Function)}.
	 */
	void putDeepCopy(UnmodifiableConfig configToCopy);

	/**
	 * Puts a deep copy of the given configuration to the root of this config.
	 * <p>
	 * This methods calls {@code deepCopy} on every sub-configuration
	 * and uses the {@code valueCopier} for other values (including lists).
	 *
	 * @param valueCopier a function that creates a deep copy of any object
	 */
	void putDeepCopy(UnmodifiableConfig configToCopy, Function<Object, Object> valueCopier);

But given that many options could be good to have, it may be better to have a dedicated class DeepConfigCopier.

@brainbytes42
Copy link
Author

The Signature doesn't fit the JavaDoc's @return...

(And I'd prefer the described functionality in @return - so that I just get a new Config-Object from an existing one, without too much hassle...)

@TheElectronWill
Copy link
Owner

TheElectronWill commented Mar 19, 2024

Ah that's right, I miscopied some code. The design choice here is: how do you choose the type of the new config. For simple configs that maps a HashMap and aren't thread-safe it's trivial, but for ConcurrentConfigs it's not (for instance, a subConfig of a SynchronizedConfig is not the same as a new SynchronizedConfig).

Thus, doing new MyNewConfig().putDeepCopy(config); works well.
An alternative would be config.deepCopyTo(MyNewConfig::new);.

@brainbytes42
Copy link
Author

Ah, I see - that makes sense...

From a usability / "discoverability" view, I'd prefer the latter, as I'd then come from an existing object and have all my methods there, with all their documentation / examples. Seems more fluent to me... But maybe that's a personal preference... :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants