Apply changes according to PR-review #191

src/ARCtrl/Templates/Templates.Json.fs

@@ -52,7 +52,7 @@ module ArcTable =
             )
         )
 
-module Json =
+module Template =
 
     open ARCtrl.ISA.Json
 

src/ARCtrl/Templates/Templates.fs

@@ -45,9 +45,9 @@ type Template(id: System.Guid, table: ArcTable, ?name: string, ?organisation: Or
     member this.SemVer
         with get() = ARCtrl.SemVer.SemVer.tryOfString this.Version
 
-    member this.structurallyEquivalent (other: Template) =
+    member this.StructurallyEquivalent (other: Template) =
         (this.Id = other.Id)
-        && (this.Table.structurallyEquivalent(other.Table))
+        && (this.Table.StructurallyEquivalent(other.Table))
         && (this.Name = other.Name)
         && (this.Organisation = other.Organisation)
         && (this.Version = other.Version)

src/ISA/ISA/ArcTypes/ArcTable.fs

@@ -7,30 +7,22 @@ open ColumnIndex
 
 open Fable.Core.JsInterop
 
-[<CustomEquality; NoComparison>]
 [<AttachMembers>]
-type ArcTable = 
-    {
-        Name : string
-        Headers : ResizeArray<CompositeHeader>
-        /// Key: Column * Row
-        Values : System.Collections.Generic.Dictionary<int*int,CompositeCell>  
-    }
+type ArcTable(name: string, headers: ResizeArray<CompositeHeader>, values: System.Collections.Generic.Dictionary<int*int,CompositeCell>) = 
 
-    static member create(name, headers, values) =
-        {
-            Name = name
-            Headers = headers
-            Values = values
+    let mutable _name = name
+    member val Headers = headers with get, set
+    member val Values = values with get, set
+    member this.Name  
+        with get() = _name
+        and internal set (newName) = _name <- newName
 
-        }
+    static member create(name, headers, values) =
+        ArcTable(name, headers, values)
 
     /// Create ArcTable with empty 'ValueHeader' and 'Values' 
-    static member init(name: string) = {
-        Name = name
-        Headers = ResizeArray<CompositeHeader>()
-        Values = System.Collections.Generic.Dictionary<int*int,CompositeCell>()
-    }
+    static member init(name: string) = 
+        ArcTable(name, ResizeArray<CompositeHeader>(), System.Collections.Generic.Dictionary<int*int,CompositeCell>())
 
     static member createFromHeaders(name,headers : ResizeArray<CompositeHeader>) =
         ArcTable.create(name,headers,Dictionary())
@@ -621,22 +613,30 @@ type ArcTable =
         ]
         |> String.concat "\n"
 
-    member this.structurallyEquivalent (other: ArcTable) =
+    member this.StructurallyEquivalent (other: ArcTable) =
         let n = this.Name = other.Name
         let headers = Aux.compareSeq this.Headers other.Headers
         let values = Aux.compareSeq (this.Values |> Seq.sortBy (fun x -> x.Key)) (other.Values |> Seq.sortBy (fun x -> x.Key))
         n && headers && values
 
+    /// <summary>
+    /// Use this function to check if this ArcTable and the input ArcTable refer to the same object.
+    ///
+    /// If true, updating one will update the other due to mutability.
+    /// </summary>
+    /// <param name="other">The other ArcTable to test for reference.</param>
+    member this.ReferenceEquals (other: ArcTable) = System.Object.ReferenceEquals(this,other)
+
     // custom check
     override this.Equals other =
         match other with
         | :? ArcTable as table -> 
-            this.structurallyEquivalent(table)
+            this.StructurallyEquivalent(table)
         | _ -> false
 
     // it's good practice to ensure that this behaves using the same fields as Equals does:
     override this.GetHashCode() = 
         let name = this.Name.GetHashCode()
         let headers = this.Headers |> Seq.fold (fun state ele -> state + ele.GetHashCode()) 0
-        let bodyCells = this.Values |> Seq.fold (fun state ele -> state + ele.GetHashCode()) 0
+        let bodyCells = this.Values |> Seq.sortBy (fun x -> x.Key) |> Seq.fold (fun state ele -> state + ele.GetHashCode()) 0
         name + headers + bodyCells

src/ISA/ISA/ArcTypes/ArcTables.fs

@@ -28,11 +28,11 @@ module ArcTablesAux =
 
     /// If a table with the given name exists in the TableList, returns it, else returns None.
     let tryFindIndexByTableName (name: string) (tables: ResizeArray<ArcTable>) =
-        Seq.tryFindIndex (fun t -> t.Name = name) tables
+        tables |> Seq.tryFindIndex (fun t -> t.Name = name)
 
     /// If a table with the given name exists in the TableList, returns it, else fails.
     let findIndexByTableName (name: string) (tables: ResizeArray<ArcTable>) =
-        match Seq.tryFindIndex (fun t -> t.Name = name) tables with
+        match tables |> Seq.tryFindIndex (fun t -> t.Name = name) with
         | Some index -> index
         | None -> failwith $"Unable to find table with name '{name}'!"
 
@@ -188,8 +188,7 @@ type ArcTables(thisTables:ResizeArray<ArcTable>) =
         SanityChecks.validateSheetIndex index false thisTables
         SanityChecks.validateNewNameUnique newName this.TableNames
         let table = this.GetTableAt index
-        let renamed = {table with Name = newName} 
-        this.UpdateTableAt(index, renamed)
+        table.Name <- newName
 
     // - Table API - //
     member this.RenameTable(name: string, newName: string) : unit =

src/ISA/ISA/ArcTypes/IdentifierSetters.fs

@@ -2,6 +2,11 @@
 
 open ARCtrl.ISA.Identifier
 
+let setArcTableName (newName: string) (table: ArcTable) =
+    checkValidCharacters newName
+    table.Name <- newName
+    table
+
 let setAssayIdentifier (newIdentifier: string) (assay: ArcAssay) =
     checkValidCharacters newIdentifier
     assay.Identifier <- newIdentifier

tests/ARCtrl/Templates.Tests.fs

@@ -132,12 +132,12 @@ let tests_Template = testList "Template" [
                 o.Table <- table
                 o.Authors <- [|ARCtrl.ISA.Person.create(FirstName="John", LastName="Doe"); ARCtrl.ISA.Person.create(FirstName="Jane", LastName="Doe");|]
                 o.EndpointRepositories <- [|ARCtrl.ISA.OntologyAnnotation.fromString "Test"; ARCtrl.ISA.OntologyAnnotation.fromString "Testing second"|]
-                let json = Encode.toString 4 (Json.encode o)
-                let actual = Decode.fromString Json.decode json
+                let json = Encode.toString 4 (Template.encode o)
+                let actual = Decode.fromString Template.decode json
                 let expected = o
                 Expect.isOk actual "Ok"
                 let actualValue = actual |> Result.toOption |> Option.get
-                Expect.isTrue (actualValue.structurallyEquivalent(expected)) "structurallyEquivalent"
+                Expect.isTrue (actualValue.StructurallyEquivalent(expected)) "structurallyEquivalent"
         ]
     ]
 

tests/ISA/ISA.Tests/ArcAssay.Tests.fs

@@ -241,7 +241,7 @@ let private tests_AddTables =
         )
         testCase "add, duplicate new names, throws" (fun () ->
             let assay = ArcAssay("MyAssay")
-            let tables = create_exampleTables("My") |> Array.map (fun x -> { x with Name = "Duplicate Name"})
+            let tables = create_exampleTables("My") |> Array.map (fun x -> x |> IdentifierSetters.setArcTableName "Duplicate Name")
             let eval() = assay.AddTables(tables)
             Expect.throws eval "throws, duplicate table names"
         )
@@ -402,7 +402,7 @@ let private tests_UpdateTableAt =
         testCase "set duplicate name" (fun () ->
             let assay = create_exampleAssay()
             let index = 2
-            let table = { create_testTable() with Name = "My Table 0" }
+            let table = create_testTable() |> IdentifierSetters.setArcTableName "My Table 0"
             let eval() = assay.UpdateTableAt(index, table)
             Expect.throws eval "throws, duplicate name"
         )

tests/ISA/ISA.Tests/ArcTable.Tests.fs

@@ -2037,6 +2037,44 @@ let private tests_UpdateRefWithSheet =
         )
     ]
 
+let private tests_equality = testList "equality" [
+    testList "override equality" [
+        testCase "equal" <| fun _ ->
+            let table1 = create_testTable()
+            let table2 = create_testTable()
+            Expect.equal table1 table2 "equal"
+        testCase "not equal" <| fun _ ->
+            let table1 = create_testTable()
+            let table2 = create_testTable()
+            table2 |> IdentifierSetters.setArcTableName "A New Name" |> ignore
+            Expect.notEqual table1 table2 "not equal"
+    ]
+    testList "structural equality" [
+        testCase "equal" <| fun _ ->
+            let table1 = create_testTable()
+            let table2 = create_testTable()
+            let equals = table1.StructurallyEquivalent(table2)
+            Expect.isTrue equals "equal"
+        testCase "not equal" <| fun _ ->
+            let table1 = create_testTable()
+            let table2 = create_testTable()
+            table2 |> IdentifierSetters.setArcTableName "A New Name" |> ignore
+            let equals = table1.StructurallyEquivalent(table2)
+            Expect.isFalse equals "not equal"
+    ]
+    testList "reference equality" [
+        testCase "not same object" <| fun _ ->
+            let table1 = create_testTable()
+            let table2 = create_testTable()
+            let equals = table1.ReferenceEquals(table2)
+            Expect.isFalse equals ""
+        testCase "same object" <| fun _ ->
+            let table1 = create_testTable()
+            let equals = table1.ReferenceEquals(table1)
+            Expect.isTrue equals ""
+    ]
+]
+
 let main = 
     testList "ArcTable" [
         tests_SanityChecks
@@ -2056,4 +2094,5 @@ let main =
         tests_AddRows
         tests_validate
         tests_UpdateRefWithSheet
+        tests_equality
     ]