Simple inheritance?

[xoofx]

Hi there!

Looking at the code, I'm trying to understand the rationale behind making a sealed Block class, use a BlockTag enum to differentiate blocks, forcing to have all datas related to a specific kind of blocks appearing on the Block class, at the same level, making the Block class almost a blob class, rather than using a simple inheritance scheme? With the current code base:

• You can't easily extend with a new kind of block (both parser and formatters)

• It makes an additional allocation of an object for each block (not a huge deal, but I'm always tracking mem alloc to avoid annoying the sensitive GC)

• It is harder to track which data is specific to a kind of blocks,

• when you read the spec, you cannot match it easily with the code, typically things like

  3.2 Container blocks and leaf blocks

  We can divide blocks into two types: container blocks, which can contain other blocks, and leaf blocks, which cannot.

If you think it would benefit the lib, would you accept a PR for this kind of refactoring? (I'm not promising anything, just taking the pulse! 😅 )

[Knagis]

I think this was already done by @dmitry-shechtman in his refactor to enable extensions.

The original idea was to match the reference C code that does not use inheritance. There are also some cases where the block changes its type that would have performed worse with different classes where the object would have to be replaced. Since the code exposes no options to extend the parsing of other blocks without changing the code, the Block and BlockTag can then be extended as well.

There shouldn't be any unused objects allocated for each block - any unused properties should be null.

[dmitry-shechtman]

I think this was already done by @dmitry-shechtman in his refactor to enable extensions.

Actually, I opted for element-specific data structs (i.e. DocumentData, EmphasisData etc.), which I believe help keep the performance hit at bay.

[xoofx]

I think this was already done by @dmitry-shechtman in his refactor to enable extensions.

Great if it is already in the pipe! Is there any branch I could look at it to see where this is going?

[dmitry-shechtman]

@xoofx Have a look at https://github.com/AMDL/CommonMark.NET/tree/pipe-tables

As already mentioned, that's still work in progress.

[xoofx]

Thanks @dmitry-shechtman I have looked at the branch. I still believe that the design of having a sealed class for block is hurting the implementation (fat class with lots of properties only valid depending on an enum, limited customization - because of the enum)

Anyway, I'm starting to play to implement from scratch a new markdown parser with pluggable blocks... so far, it is going well, but damn, commonmark specs are quite difficult to implement correctly!