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

Fixes #2161: #2162

Merged
merged 2 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/OSPSuite.Assets/UIConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -583,16 +583,16 @@ public static class Diff
public static readonly string ObjectPath = "Path";
public static readonly string ExcludeMolecule = "Exclude Molecule";
public static readonly string IncludeMolecule = "Include Molecule";
public static readonly string Count = "Number of item";
public static readonly string Assignment = "Assignment";
public static readonly string OneObjectIsNull = "One object used in the comparison is null";
public static readonly string Connection = "Connection";
public static readonly string SourceAmount = "Source Amount";
public static readonly string TargetAmount = "Target Amount";

public static string PropertyDiffers(string propertyName, string value1, string value2)
{
return $"{propertyName.Pluralize()} are not equal ({value1} ≠ {value2})";
}
public static string PropertyDiffers(string propertyName, int value1, int value2) => PropertyDiffers(propertyName, $"{value1}", $"{value2}");

public static string PropertyDiffers(string propertyName, string value1, string value2) => $"{propertyName.Pluralize()} are not equal ({value1} ≠ {value2})";

public static string DifferentTypes(string type1, string type2)
{
Expand Down
47 changes: 39 additions & 8 deletions src/OSPSuite.Core/Comparison/EnumerableComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using OSPSuite.Assets;
using OSPSuite.Core.Domain.Services;
using OSPSuite.Core.Services;
using OSPSuite.Utility.Extensions;

namespace OSPSuite.Core.Comparison
{
Expand Down Expand Up @@ -59,6 +60,39 @@ public void CompareEnumerables<TParent, TItem, TResult>(IComparison<TParent> com
);
}

public void CompareEnumerablesByIndex<TParent, TItem>(IComparison<TParent> comparison,
Func<TParent, IEnumerable<TItem>> getEnumeration)
where TParent : class
where TItem : class
{
var object1 = comparison.Object1;
var object2 = comparison.Object2;
var list1 = getEnumeration(object1).ToList();
var list2 = getEnumeration(object2).ToList();

if (list1.Count != list2.Count)
{
comparison.Add( new PropertyValueDiffItem
{
Object1 = comparison.Object1,
Object2 = comparison.Object2,
CommonAncestor = comparison.CommonAncestor,
PropertyName = Captions.Diff.Count,
FormattedValue1 = $"{list1.Count}",
FormattedValue2 = $"{list2.Count}",
Description = Captions.Diff.PropertyDiffers(Captions.Diff.Count, list1.Count, list2.Count)
});
return;
}

list1.Each((entity1, index) =>
{
var entity2 = list2[index];
var childComparison = new Comparison<TItem>(entity1, entity2, comparison.Settings, comparison.Report, object1);
_objectComparer.Compare(childComparison);
});
}

public void CompareEnumerables<TParent, TItem>(IComparison<TParent> comparison,
Func<TParent, IEnumerable<TItem>> getEnumeration,
Func<TItem, TItem, bool> equalityFunc,
Expand All @@ -72,6 +106,9 @@ public void CompareEnumerables<TParent, TItem>(IComparison<TParent> comparison,
var object2 = comparison.Object2;
var list1 = getEnumeration(object1).ToList();
var list2 = getEnumeration(object2).ToList();
string getMissingType(TItem item) => missingItemType ?? _objectTypeResolver.TypeFor(item);
string getDetails(TItem item) => presentObjectDetailsFunc?.Invoke(item) ?? string.Empty;


foreach (var entity1 in list1)
{
Expand All @@ -82,11 +119,7 @@ public void CompareEnumerables<TParent, TItem>(IComparison<TParent> comparison,
_objectComparer.Compare(childComparison);
}
else
{
var missingObjectType = missingItemType ?? _objectTypeResolver.TypeFor(entity1);
var presentObjectDetails = presentObjectDetailsFunc?.Invoke(entity1) ?? string.Empty;
comparison.Add(missingItem(object1, object2, entity1, null, missingObjectType, identifierRetrieverFunc(entity1), presentObjectDetails));
}
comparison.Add(missingItem(object1, object2, entity1, null, getMissingType(entity1), identifierRetrieverFunc(entity1), getDetails(entity1)));
}

//all common entity have been added. Add missing entity from object1 base on object2
Expand All @@ -95,9 +128,7 @@ public void CompareEnumerables<TParent, TItem>(IComparison<TParent> comparison,
if (list1.Any(item => equalityFunc(item, entity2)))
continue;

var missingObjectType = missingItemType ?? _objectTypeResolver.TypeFor(entity2);
var presentObjectDetails = presentObjectDetailsFunc?.Invoke(entity2) ?? string.Empty;
comparison.Add(missingItem(object1, object2, null, entity2, missingObjectType, identifierRetrieverFunc(entity2), presentObjectDetails));
comparison.Add(missingItem(object1, object2, null, entity2, getMissingType(entity2), identifierRetrieverFunc(entity2), getDetails(entity2)));
}
}

Expand Down
19 changes: 4 additions & 15 deletions src/OSPSuite.Core/Comparison/TableFormulaDiffBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
using OSPSuite.Assets;
using OSPSuite.Core.Domain.Formulas;
using OSPSuite.Core.Domain.Services;
using OSPSuite.Core.Domain.Formulas;
using OSPSuite.Core.Services;
using OSPSuite.Utility.Extensions;

namespace OSPSuite.Core.Comparison
{
Expand All @@ -11,14 +8,12 @@ public class TableFormulaDiffBuilder : DiffBuilder<TableFormula>
private readonly IObjectComparer _objectComparer;
private readonly ObjectBaseDiffBuilder _objectBaseDiffBuilder;
private readonly EnumerableComparer _enumerableComparer;
private readonly IDisplayNameProvider _displayNameProvider;

public TableFormulaDiffBuilder(IObjectComparer objectComparer, ObjectBaseDiffBuilder objectBaseDiffBuilder, EnumerableComparer enumerableComparer, IDisplayNameProvider displayNameProvider)
public TableFormulaDiffBuilder(IObjectComparer objectComparer, ObjectBaseDiffBuilder objectBaseDiffBuilder, EnumerableComparer enumerableComparer)
{
_objectComparer = objectComparer;
_objectBaseDiffBuilder = objectBaseDiffBuilder;
_enumerableComparer = enumerableComparer;
_displayNameProvider = displayNameProvider;
}

public override void Compare(IComparison<TableFormula> comparison)
Expand All @@ -28,14 +23,8 @@ public override void Compare(IComparison<TableFormula> comparison)
CompareStringValues(x => x.XName, x => x.XName, comparison);
CompareStringValues(x => x.YName, x => x.YName, comparison);
CompareValues(x => x.UseDerivedValues, x => x.UseDerivedValues, comparison);
_enumerableComparer.CompareEnumerables(comparison, x => x.AllPoints(), (pt1, pt2) => ValueComparer.AreValuesEqual(pt1.X, pt2.X, comparison.Settings.RelativeTolerance), x => displayValueFor(x, comparison));
}

private string displayValueFor(ValuePoint valuePoint, IComparison<TableFormula> comparison)
{
var formulaOwnerName = _displayNameProvider.DisplayNameFor(comparison.CommonAncestor);
var tableFormula = comparison.Object1.DowncastTo<TableFormula>();
return Captions.Comparisons.ValuePointAt(tableFormula.Name, formulaOwnerName, tableFormula.XDisplayValueFor(valuePoint.X));
//We do not use enumerable comparison here as we need to compare the values by index
_enumerableComparer.CompareEnumerablesByIndex(comparison, x => x.AllPoints);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we compare by index. That way comparison of tables with two points with very close x axis does not fail for no reason

}
}
}
8 changes: 4 additions & 4 deletions src/OSPSuite.Core/Domain/Formulas/TableFormula.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public ValuePoint(double x, double y)
public class TableFormula : Formula
{
private readonly IInterpolation _interpolation;
private readonly IList<ValuePoint> _allPoints;
private readonly List<ValuePoint> _allPoints;
private Unit _xDisplayUnit;
private Unit _yDisplayUnit;

Expand Down Expand Up @@ -88,9 +88,9 @@ public TableFormula(IInterpolation interpolation)
UseDerivedValues = true;
}

public virtual IEnumerable<ValuePoint> AllPoints() => _allPoints;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just garbage

public virtual IReadOnlyList<ValuePoint> AllPoints => _allPoints;

public virtual ValuePoint[] AllPointsAsArray() => AllPoints().ToArray();
public virtual ValuePoint[] AllPointsAsArray() => AllPoints.ToArray();

/// <summary>
/// Returns the yValue defined for the xValue in base unit given as parameter. If the table contains no point, 0 is
Expand Down Expand Up @@ -283,7 +283,7 @@ public override void UpdatePropertiesFrom(IUpdatable source, ICloneManager clone
_yDisplayUnit = tableFormula._yDisplayUnit;

ClearPoints();
tableFormula.AllPoints().Each(p => AddPoint(p.Clone()));
tableFormula.AllPoints.Each(p => AddPoint(p.Clone()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ public DataRepository MapFrom(TableFormula tableFormula)
var baseGrid = new BaseGrid(_idGenerator.NewId(), tableFormula.XDimension.Name, tableFormula.XDimension) {DataInfo = {DisplayUnitName = tableFormula.XDisplayUnit.Name}};
var value = new DataColumn(_idGenerator.NewId(), tableFormula.Name, tableFormula.Dimension, baseGrid) {DataInfo = {DisplayUnitName = tableFormula.YDisplayUnit.Name}};

baseGrid.Values = tableFormula.AllPoints().Select(x => x.X).ToFloatArray();
value.Values = tableFormula.AllPoints().Select(x => x.Y).ToFloatArray();
baseGrid.Values = tableFormula.AllPoints.Select(x => x.X).ToFloatArray();
value.Values = tableFormula.AllPoints.Select(x => x.Y).ToFloatArray();

dataRepository.Add(value);
return dataRepository;
Expand Down
2 changes: 1 addition & 1 deletion src/OSPSuite.Core/Domain/Services/ModelReportCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ private void reportFor(IFormula formula, string caption, int noOfTabs)

private void reportFor(TableFormula tableFormula, int noOfTabs)
{
foreach (var valuePoint in tableFormula.AllPoints())
foreach (var valuePoint in tableFormula.AllPoints)
{
_report.AppendFormat("{0}{1}; {2} RestartSolver = {3}", tabs(noOfTabs), valuePoint.X, valuePoint.Y, valuePoint.RestartSolver);
_report.AppendLine();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class TableFormulaToTableFormulaExportMapper : ITableFormulaToTableFormulaExport
public TableFormulaExport MapFrom(TableFormula input)
{
var export = new TableFormulaExport();
input.AllPoints().Each(export.AddPoint);
input.AllPoints.Each(export.AddPoint);

export.UseDerivedValues = input.UseDerivedValues;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public override void PerformMapping()
Map(x => x.XDimension).WithMappingName(Constants.Serialization.Attribute.X_DIMENSION);
Map(x => x.XName);
Map(x => x.YName);
MapEnumerable(x => x.AllPoints(), addPoint);
MapEnumerable(x => x.AllPoints, addPoint);
Map(x => x.UseDerivedValues);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,34 @@ public void should_report_the_path_entries_and_different_entries_as_difference()
_report.Count.ShouldBeEqualTo(5);
}
}
}

public class When_comparing_two_table_formula_with_different_number_of_points : concern_for_ObjectComparer
{
protected override void Context()
{
base.Context();
var parameter1 = new Parameter().WithName("P");
var formula1 = new TableFormula();
formula1.AddPoint(10, 1);
formula1.AddPoint(20, 2);
formula1.AddPoint(30, 3);
parameter1.Formula = formula1;

var parameter2 = new Parameter().WithName("P");
var formula2 = new TableFormula();
formula2.AddPoint(10, 1);
formula2.AddPoint(20, 2);
parameter2.Formula = formula2;

_object1 = parameter1;
_object2 = parameter2;
}

[Observation]
public void should_report_a_difference()
{
_report.Count.ShouldBeEqualTo(1);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ public static void AreEqualTableFormula(TableFormula x1, TableFormula x2)
AreEqualUnit(x1.YDisplayUnit, x2.YDisplayUnit);
Assert.AreEqual(x1.XName, x2.XName);
Assert.AreEqual(x1.YName, x2.YName);
AreEqualEnumerableWithSameOrder(x1.AllPoints(), x2.AllPoints());
AreEqualEnumerableWithSameOrder(x1.AllPoints, x2.AllPoints);
}

public static void AreEqualExplicitFormula(ExplicitFormula x1, ExplicitFormula x2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ public void TestSerialization()

var x2 = SerializeAndDeserialize(x1);

x2.AllPoints().Count().ShouldBeEqualTo(2);
x2.AllPoints.Count().ShouldBeEqualTo(2);

x2.AllPoints().ElementAt(0).X.ShouldBeEqualTo(0);
x2.AllPoints().ElementAt(0).Y.ShouldBeEqualTo(1);
x2.AllPoints.ElementAt(0).X.ShouldBeEqualTo(0);
x2.AllPoints.ElementAt(0).Y.ShouldBeEqualTo(1);

x2.AllPoints().ElementAt(1).X.ShouldBeEqualTo(2);
x2.AllPoints().ElementAt(1).Y.ShouldBeEqualTo(3);
x2.AllPoints.ElementAt(1).X.ShouldBeEqualTo(2);
x2.AllPoints.ElementAt(1).Y.ShouldBeEqualTo(3);

x2.AllDistributionMetaData().Count().ShouldBeEqualTo(2);
var distributionMetaData = x2.AllDistributionMetaData().ElementAt(0);
Expand Down
12 changes: 6 additions & 6 deletions tests/OSPSuite.Core.Tests/Domain/TableFormulaSpecs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ protected override void Context()
[Observation]
public void should_return_them_sorted_by_time()
{
sut.AllPoints().ShouldOnlyContainInOrder(_p1, _p2, _p3);
sut.AllPoints.ShouldOnlyContainInOrder(_p1, _p2, _p3);
}
}

Expand All @@ -119,7 +119,7 @@ protected override void Context()
[Observation]
public void should_not_add_the_same_point_twice()
{
sut.AllPoints().ShouldOnlyContainInOrder(_p1, _p2);
sut.AllPoints.ShouldOnlyContainInOrder(_p1, _p2);
}
}

Expand Down Expand Up @@ -163,7 +163,7 @@ protected override void Because()
[Observation]
public void the_table_formula_should_not_contain_any_points()
{
sut.AllPoints().Count().ShouldBeEqualTo(0);
sut.AllPoints.Count().ShouldBeEqualTo(0);
}
}

Expand Down Expand Up @@ -213,7 +213,7 @@ protected override void Because()
[Observation]
public void should_have_removed_that_point()
{
sut.AllPoints().ShouldOnlyContain(_p2);
sut.AllPoints.ShouldOnlyContain(_p2);
}
}

Expand All @@ -240,7 +240,7 @@ protected override void Because()
[Observation]
public void should_have_removed_that_point()
{
sut.AllPoints().ShouldOnlyContain(_p2);
sut.AllPoints.ShouldOnlyContain(_p2);
}
}

Expand All @@ -267,7 +267,7 @@ protected override void Because()
[Observation]
public void should_not_remove_the_existing_points()
{
sut.AllPoints().ShouldOnlyContain(_p1, _p2);
sut.AllPoints.ShouldOnlyContain(_p1, _p2);
}
}

Expand Down