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

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

Open
pietroppeter opened this issue Jan 29, 2023 · 6 comments
Open
Assignees
Labels

Comments

@pietroppeter
Copy link
Owner

pietroppeter commented Jan 29, 2023

  • 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 pietroppeter self-assigned this Jan 29, 2023
@HugoGranstrom
Copy link
Collaborator

HugoGranstrom commented Feb 21, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@pietroppeter
Copy link
Owner Author

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
Copy link
Owner Author

some discussion here: nimib-land/nblog#20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants