nimib icon indicating copy to clipboard operation
nimib copied to clipboard

refactor NbBlock to use OOP (hierarchy of objects / inheritance)

Open pietroppeter opened this issue 2 years ago • 6 comments
trafficstars

  • in the past we were not fan of OOP but that was probably not a good take
  • indeed we now think that for NbBlock it could make a lot of sense to have a base object and a hierarchy of objects that inherit from it
  • still need to make some experience in using OOP stuff (maybe in a nimib not related lib)

pietroppeter avatar Jan 29 '23 09:01 pietroppeter

One problem we discussed in a past Speaking Hour is the fact that we must be able to serialize all our objects to JSON. The problem being that we don't have static knowledge of the exact type of each block. If we use the dynamic dispatch functionality of Nim, we can create a dumpHook for the base type and then call a method that returns the string of the specific type:

import jsony

type
  Nimib = ref object of RootObj
    a, b, c: int
  NimibChild = ref object of Nimib
    d, e: float


method dump(n: Nimib): string =
  n[].toJson()

method dump(n: NimibChild): string =
  n[].toJson()

proc dumpHook*(s: var string, v: Nimib) =
  s.add v.dump()

let n1: Nimib = Nimib(a: 1, b: 2, c: 3)
let n2: Nimib = NimibChild(d: 3.14, e: 4.56)

echo n1.toJson()
echo n2.toJson()

The next problem then becomes how we parse the JSON again without losing the type information...

HugoGranstrom avatar Feb 21 '24 18:02 HugoGranstrom

Here is a solution that works:

import jsony, tables

type
  Nimib = ref object of RootObj
    a, b, c: int
    typename: string
  NimibChild = ref object of Nimib
    d, e: float

method dump(n: Nimib): string =
  n[].toJson()

method dump(n: NimibChild): string =
  n[].toJson()

proc dumpHook*(s: var string, v: Nimib) =
  s.add v.dump()

let n1: Nimib = Nimib(a: 1, b: 2, c: 3, typename: "Nimib")
let n2: Nimib = NimibChild(a: 100, d: 3.14, e: 4.56, typename: "NimibChild")

echo n1.toJson()
echo n2.toJson()

proc parseNimib(s: string, i: var int): Nimib =
  var v = Nimib()
  parseHook(s, i, v[])
  result = v

proc parseNimibChild(s: string, i: var int): Nimib =
  var v = NimibChild()
  parseHook(s, i, v[])
  result = v

let parseDefs = {
  "Nimib": parseNimib,
  "NimibChild": parseNimibChild
}.toTable()

proc parseHook*(s: string, i: var int, v: var Nimib) =
  var n: Nimib = Nimib()
  let current_i = i
  parseHook(s, i, n[])
  i = current_i
  let typename = n.typename
  v = parseDefs[typename](s, i)

let s = n2.toJson().fromJson(Nimib)
echo s.toJson()

We store the type name as a string in the mandatory field typename and add a proc for each type name in a table. Then we can look up the correct proc when parsing the JSON in parseHook. The downside of this is that we must construct the table and know all the blocks that exists. But this could be solved by requiring blocks to be "registered". The act of registering a block could then generate all the above necessary proc/methods automatically.

This is a lot to take in, but it works.

HugoGranstrom avatar Feb 21 '24 18:02 HugoGranstrom

And here the template version is:

import jsony, tables

type
  Nimib = ref object of RootObj
    a, b, c: int
    typename: string
  NimibChild = ref object of Nimib
    d, e: float

var parseDefs: Table[string, proc (s: string, i: var int): Nimib]

template registerBlock(typename: untyped) =
  parseDefs[$typename] =
    proc (s: string, i: var int): Nimib =
      var v: typename
      new v
      parseHook(s, i, v[])
      result = v

  method dump(n: typename): string =
    n[].toJson()

# neccecary to avoid compile error
method dump(n: Nimib): string =
    n[].toJson()

proc dumpHook*(s: var string, v: Nimib) =
  s.add v.dump()

proc parseHook*(s: string, i: var int, v: var Nimib) =
  # First parse the typename
  var n: Nimib = Nimib()
  let current_i = i
  parseHook(s, i, n[])
  # Reset i
  i = current_i
  # Parse the correct type
  let typename = n.typename
  v = parseDefs[typename](s, i)

registerBlock(Nimib)
registerBlock(NimibChild)

let n1: Nimib = Nimib(a: 1, b: 2, c: 3, typename: "Nimib")
let n2: Nimib = NimibChild(a: 100, d: 3.14, e: 4.56, typename: "NimibChild")

echo n1.toJson()
echo n2.toJson()

let s = n2.toJson().fromJson(Nimib)
echo s.toJson()

It's just one extra line of code for a block developer with this approach.

HugoGranstrom avatar Feb 21 '24 19:02 HugoGranstrom

With this, I think we might actually be able to start working for real on a refactoring of Nimib.

HugoGranstrom avatar Feb 21 '24 19:02 HugoGranstrom

Wow this looks very interesting, thanks a lot! I will have to look at it more closely and try it out! Will let you know my thoughts!

pietroppeter avatar Feb 22 '24 04:02 pietroppeter

some discussion here: https://github.com/nimib-land/nblog/issues/20

pietroppeter avatar Feb 25 '24 09:02 pietroppeter