From a12e9c2637548097e7cfc433a4a6bfad22c01327 Mon Sep 17 00:00:00 2001 From: Steffen Schaefer Date: Tue, 7 Feb 2017 11:09:56 +0100 Subject: [PATCH] Calling overwritable methods during object construction is an antipattern. Using on the fly object creation instead. --- .../base/AbstractValueCollectionSetting.java | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java index 041c796c762..104f1c107d6 100644 --- a/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java +++ b/java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/base/AbstractValueCollectionSetting.java @@ -12,7 +12,7 @@ import com.sap.sse.common.settings.value.ValueCollectionValue; public abstract class AbstractValueCollectionSetting, D extends Collection> extends AbstractHasValueSetting implements ValueCollectionSetting { - private final D defaultValues = createDefaultValuesCollection(); + private D defaultValues; private final boolean emptyIsDefault; public AbstractValueCollectionSetting(String name, AbstractGenericSerializableSettings settings, ValueConverter valueConverter, boolean emptyIsDefault) { @@ -20,6 +20,13 @@ public abstract class AbstractValueCollectionSetting getValue() { return (ValueCollectionValue) settings.getValue(settingName); @@ -42,7 +49,7 @@ public abstract class AbstractValueCollectionSetting getValues() { ValueCollectionValue value = getValue(); if(emptyIsDefault && (value == null || value.isEmpty())) { - return Collections.unmodifiableCollection(defaultValues); + return Collections.unmodifiableCollection(getDefaultValuesCollectionInternal()); } if(value == null) { return Collections.emptyList(); @@ -70,22 +77,24 @@ public abstract class AbstractValueCollectionSetting value = getValue(); - return ((emptyIsDefault || (defaultValues == null || defaultValues.isEmpty())) && (value == null || value.isEmpty())) - || (value != null && value.size() == defaultValues.size() && defaultValues.containsAll(value.getValues(getValueConverter()))); + final ValueCollectionValue value = getValue(); + final D defaultVal = getDefaultValuesCollectionInternal(); + return ((emptyIsDefault || defaultVal.isEmpty()) && (value == null || value.isEmpty())) + || (value != null && value.size() == defaultVal.size() && defaultVal.containsAll(value.getValues(getValueConverter()))); } @Override public final void setDefaultValues(Iterable defaultValues) { - boolean wasDefault = isDefaultValue(); - this.defaultValues.clear(); + final boolean wasDefault = isDefaultValue(); + final D defaultVal = getDefaultValuesCollectionInternal(); + defaultVal.clear(); if(defaultValues != null) { - Util.addAll(defaultValues, this.defaultValues); + Util.addAll(defaultValues, defaultVal); } if(wasDefault) { resetToDefault(); @@ -94,7 +103,7 @@ public abstract class AbstractValueCollectionSetting getDefaultValues() { - return Collections.unmodifiableCollection(this.defaultValues); + return Collections.unmodifiableCollection(getDefaultValuesCollectionInternal()); } @Override