Skip to content

Commit

Permalink
Fix code smells; Fix #395 (#403)
Browse files Browse the repository at this point in the history 
* Fix some of the js code smells

* Add filtering possibility to record changes

* ParameterEditorBody reuse parent logic

* Fix build failure after rebase

* bug fix

---------

Co-authored-by: antoineatrhea <[email protected]>
  • Loading branch information
@jaimeatstariongroup
jaimeatstariongroup and antoineatrhea authored Aug 16, 2023
1 parent dd383f4 commit dcdc296
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 104 deletions.
17 changes: 16 additions & 1 deletion COMET.Web.Common/Utilities/HaveObjectChangedTracking/HaveObjectChangedTracking.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,27 @@ protected void ClearRecordedChanges()
this.AddedThings.Clear();
}

/// <summary>
/// The logic used to check if a change should be recorded an <see cref="ObjectChangedEvent"/>
/// </summary>
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent"/></param>
/// <returns>true if the change should be recorded, false otherwise</returns>
protected virtual bool ShouldRecordChange(ObjectChangedEvent objectChangedEvent)
{
return true;
}

/// <summary>
/// Records an <see cref="ObjectChangedEvent" />
/// </summary>
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent" /></param>
protected virtual void RecordChange(ObjectChangedEvent objectChangedEvent)
private void RecordChange(ObjectChangedEvent objectChangedEvent)
{
if (!this.ShouldRecordChange(objectChangedEvent))
{
return;
}

switch (objectChangedEvent.EventKind)
{
case EventKind.Added:
Expand Down
29 changes: 15 additions & 14 deletions COMET.Web.Common/ViewModels/Components/SingleIterationApplicationBaseViewModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,6 @@ public Iteration CurrentIteration
/// </summary>
public bool HasSetInitialValuesOnce { get; set; }

/// <summary>
/// Records an <see cref="ObjectChangedEvent" />
/// </summary>
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent" /></param>
protected override void RecordChange(ObjectChangedEvent objectChangedEvent)
{
if (this.CurrentIteration == null || objectChangedEvent.ChangedThing.GetContainerOfType<Iteration>().Iid != this.CurrentIteration.Iid)
{
return;
}

base.RecordChange(objectChangedEvent);
}

/// <summary>
/// Handles the refresh of the current <see cref="ISession" />
/// </summary>
Expand All @@ -147,5 +133,20 @@ protected virtual async Task OnIterationChanged()
this.CurrentDomain = this.CurrentIteration == null ? null : this.SessionService.GetDomainOfExpertise(this.CurrentIteration);
await Task.CompletedTask;
}

/// <summary>
/// The logic used to check if a change should be recorded an <see cref="ObjectChangedEvent"/>
/// </summary>
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent"/></param>
/// <returns>true if the change should be recorded, false otherwise</returns>
protected override bool ShouldRecordChange(ObjectChangedEvent objectChangedEvent)
{
if (this.CurrentIteration == null || objectChangedEvent.ChangedThing.GetContainerOfType<Iteration>().Iid != this.CurrentIteration.Iid)
{
return false;
}

return true;
}
}
}
80 changes: 14 additions & 66 deletions COMETwebapp/ViewModels/Components/ParameterEditor/ParameterEditorBodyViewModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@

namespace COMETwebapp.ViewModels.Components.ParameterEditor
{
using System.Reactive.Linq;

using CDP4Common.CommonData;
using CDP4Common.EngineeringModelData;
using CDP4Common.SiteDirectoryData;

Expand All @@ -47,26 +44,21 @@ namespace COMETwebapp.ViewModels.Components.ParameterEditor
/// </summary>
public class ParameterEditorBodyViewModel : SingleIterationApplicationBaseViewModel, IParameterEditorBodyViewModel
{
/// <summary>
/// A collection of added <see cref="Thing" />s
/// </summary>
private readonly List<Thing> addedThings = new();

/// <summary>
/// A collection of deleted <see cref="Thing" />s
/// </summary>
private readonly List<Thing> deletedThings = new();

/// <summary>
/// Backing field for the <see cref="IsOwnedParameters" />
/// </summary>
private bool isOwnedParameters;

/// <summary>
/// A collection of updated <see cref="Thing" />s
/// A collection of <see cref="Type" /> used to create <see cref="ObjectChangedEvent" /> subscriptions
/// </summary>
private readonly List<Thing> updatedThings = new();

private static readonly IEnumerable<Type> ObjectChangedTypesOfInterest = new List<Type>
{
typeof(ElementBase),
typeof(ParameterOrOverrideBase),
typeof(ParameterValueSetBase),
};

/// <summary>
/// Creates a new instance of <see cref="ParameterEditorBodyViewModel" />
/// </summary>
Expand All @@ -84,14 +76,7 @@ public ParameterEditorBodyViewModel(ISessionService sessionService, ISubscriptio
x => x.ParameterTypeSelector.SelectedParameterType,
x => x.IsOwnedParameters).SubscribeAsync(_ => this.ApplyFilters()));

var observables = new List<IObservable<ObjectChangedEvent>>
{
CDPMessageBus.Current.Listen<ObjectChangedEvent>(typeof(ParameterValueSetBase)),
CDPMessageBus.Current.Listen<ObjectChangedEvent>(typeof(ElementBase)),
CDPMessageBus.Current.Listen<ObjectChangedEvent>(typeof(ParameterOrOverrideBase))
};

this.Disposables.Add(observables.Merge().Subscribe(this.RecordChange));
this.InitializeSubscriptions(ObjectChangedTypesOfInterest);
}

/// <summary>
Expand Down Expand Up @@ -134,17 +119,17 @@ public bool IsOwnedParameters
/// <returns>A <see cref="Task" /></returns>
protected override async Task OnSessionRefreshed()
{
if (!this.addedThings.Any() && !this.deletedThings.Any() && !this.updatedThings.Any())
if (!this.AddedThings.Any() && !this.DeletedThings.Any() && !this.UpdatedThings.Any())
{
return;
}

this.IsLoading = true;
await Task.Delay(1);
this.ParameterTableViewModel.RemoveRows(this.deletedThings.ToList());
this.ParameterTableViewModel.UpdateRows(this.updatedThings.ToList());
this.ParameterTableViewModel.AddRows(this.addedThings.ToList());
this.ClearRecordedChange();
this.ParameterTableViewModel.RemoveRows(this.DeletedThings.ToList());
this.ParameterTableViewModel.UpdateRows(this.UpdatedThings.ToList());
this.ParameterTableViewModel.AddRows(this.AddedThings.ToList());
this.ClearRecordedChanges();
this.IsLoading = false;
}

Expand Down Expand Up @@ -185,43 +170,6 @@ protected override async Task OnIterationChanged()
await this.InitializeTable();
}

/// <summary>
/// Clears all recorded changed
/// </summary>
private void ClearRecordedChange()
{
this.deletedThings.Clear();
this.updatedThings.Clear();
this.addedThings.Clear();
}

/// <summary>
/// Records an <see cref="ObjectChangedEvent" />
/// </summary>
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent" /></param>
protected override void RecordChange(ObjectChangedEvent objectChangedEvent)
{
if (this.CurrentIteration == null || objectChangedEvent.ChangedThing.GetContainerOfType<Iteration>().Iid != this.CurrentIteration.Iid)
{
return;
}

switch (objectChangedEvent.EventKind)
{
case EventKind.Added:
this.addedThings.Add(objectChangedEvent.ChangedThing);
break;
case EventKind.Removed:
this.deletedThings.Add(objectChangedEvent.ChangedThing);
break;
case EventKind.Updated:
this.updatedThings.Add(objectChangedEvent.ChangedThing);
break;
default:
throw new ArgumentOutOfRangeException(nameof(objectChangedEvent), "Unrecognised value EventKind value");
}
}

/// <summary>
/// Initialize the <see cref="IParameterTableViewModel" />
/// </summary>
Expand Down
22 changes: 5 additions & 17 deletions COMETwebapp/ViewModels/Components/UserManagement/UserManagementTableViewModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,25 +111,13 @@ protected override async Task OnSessionRefreshed()
}

/// <summary>
/// Records an <see cref="ObjectChangedEvent" />
/// The logic used to check if a change should be recorded an <see cref="ObjectChangedEvent"/>
/// </summary>
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent" /></param>
protected override void RecordChange(ObjectChangedEvent objectChangedEvent)
/// <param name="objectChangedEvent">The <see cref="ObjectChangedEvent"/></param>
/// <returns>true if the change should be recorded, false otherwise</returns>
protected override bool ShouldRecordChange(ObjectChangedEvent objectChangedEvent)
{
switch (objectChangedEvent.EventKind)
{
case EventKind.Added:
this.AddedThings.Add(objectChangedEvent.ChangedThing);
break;
case EventKind.Removed:
this.DeletedThings.Add(objectChangedEvent.ChangedThing);
break;
case EventKind.Updated:
this.UpdatedThings.Add(objectChangedEvent.ChangedThing);
break;
default:
throw new ArgumentOutOfRangeException(nameof(objectChangedEvent), "Unrecognised value EventKind value");
}
return true;
}

/// <summary>
Expand Down
5 changes: 2 additions & 3 deletions COMETwebapp/wwwroot/Scripts/babylonInterop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ let HighLightLayer;
* Inits the babylon.js scene on the canvas, the asociated resources and starts the render loop.
* @param {HTMLCanvasElement} canvas - the canvas the scene it's attached to.
*/
function InitCanvas(canvas,addAxes) {
function InitCanvas(canvas, addAxes) {

if (canvas == null || canvas == undefined) {
if (canvas == null) {
throw "The canvas can't be null or undefined";
}

Expand All @@ -136,7 +136,6 @@ function InitCanvas(canvas,addAxes) {
CreateSkybox(Scene, SkyboxSize);

PickingMaterial = SetUpPickingMaterial();
DetailsPanel = document.getElementById("detailsPanel");

SceneSpecularColor = new BABYLON.Color3(1.0, 1.0, 1.0);
SceneEmissiveColor = new BABYLON.Color3(0.0, 0.0, 0.0);
Expand Down
7 changes: 4 additions & 3 deletions COMETwebapp/wwwroot/Scripts/babylonSpecifics.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ function CreateScene(engine, canvas) {
Camera.panningSensibility = CameraPanningSensibility;
Camera.wheelPrecision = CameraZoomSensibility;

let light1 = new BABYLON.HemisphericLight("HemisphericLight", new BABYLON.Vector3(2, 1, 0), scene);
let light = new BABYLON.HemisphericLight("HemisphericLight", new BABYLON.Vector3(2, 1, 0));
scene.light = light;

return scene;
};
Expand Down Expand Up @@ -190,8 +191,8 @@ async function LoadPrimitive(primitive) {
const result = await BABYLON.SceneLoader.ImportMeshAsync(null, path, fileName, Scene);
let meshes = result.meshes;

for (let i = 0; i < meshes.length; i++) {
InitializePrimitiveData(meshes[i], primitive);
for (let mesh of meshes) {
InitializePrimitiveData(mesh, primitive);
}
}

Expand Down

0 comments on commit dcdc296

Please sign in to comment.