Concrete classes vs. abstract classes / trait

[jrudolph]

It may have been discussed before (I skimmed through the gitter archive but didn't find anything) but I wonder if an abstract class / trait based approach would be feasible as well.

I've tried to come up some advantages / disadvantages (without giving them any priority or evaluation at this point):

Concrete AST

Advantages

• there's only one AST and the AST is only somewhat usable without any library
• speed, if there's only one concrete implementation available, all call-sites should be fast and inlinable

Disadvantages

• if the AST doesn't provide enough useful functions, an implementation needs to wrap the AST or just adapt to it which defeats its purpose (though wrapping is free if it can be provided as a value class)
• if an implementation wants to add additional fields it needs a wrapper, e.g. if an implementation wants to only support one kind of number for optimization purposes, adding a field wouldn't be possible
• the current split between "fast" and "safe" already introduces a second kind of AST, which means there is no canonical implementation anyway

Abstract AST - Implementations provide concrete implementation

Advantages

• Implementations could provide whatever implementations they like without wrapping
• The AST could be minimal set of interfaces / abstract classes.

Disadvantages

• as concrete implementations are missing, the AST itself would not be useful
• if pattern matching / extraction should be a feature of the AST, its implementation will be more complicated if no case classes can be used
• a slight method invocation cost if several json libraries are used that provide implementations at the same time (multimorphic dispatch)

[mdedetrich]

One of the intentions I had in mind (at least with safe), is that when a user receives a safe.JValue, they have a guarantee that its a proper valid JSON value. This is one of the advantage that a Concrete AST provides in this area. If you have an abstract AST which lets parsers be able to construct freely their AST, you give the ability for JSON library authors to provide an invalid JSON.

Also, unless I am mistaken, you should be able to extend a concrete AST fine using implicit classes

Also agree we should @inline the methods, there was some valid reasoning against this before, but if its going to be a scala module, there is very little downside to this

[jrudolph]

I think the best reason for the abstract class based variant would be that it would be trivial to adopt. In the first version, you would just add those classes into your hierarchy and make sure to implement the required methods. Then, you would change your library to accept the common types everywhere instead of your specialized ones.

[mdedetrich]

The biggest risk with concrete classes, honestly, is choosing the wrong data structures. It does seem that other languages do fine in this area though (in fact, data structures are the main reason we have the safe/fast split)

I mean you could do a sought of hybrid approach, where you have an Abstract AST, however it requires the users to provide types for the data structures they want to implement. Something like

// Assuming we have something like
trait JValue[JNumberType,JArrayType <: Seq,JObjectType] {
// ...
}

type SafeJValue = JValue[BigNumber,Vector,Map[String,JValue[_,_,_]] // Equivalent to current safe
type FastJValue = JValue[String,Array,Array[JField[_,_,_]] // Equivalent to current fast

JValue[BigNumber,List,List[JField[_,_,_]]] // Some custom implementation

JValue[Decimal,List,List[JField[_,_,_]]] // Another custom implementation

That way, if a user asks for a JsonAST[BigNumber,Vector,Map] (which the parser would need to provide), due to the data-structures that were chosen, the user has a guarantee that JSON is valid

The abstract AST can also provide type converters for common representations (i.e. going from JsonAST[BigNumber,Vector,Map] to a JsonAST[String,Array,Array]) as we do currently, without having to provide an actual implementation (which is what a parser would do).

It also gives parser the ability to provide custom adapters for types which they don't parse into yet

I would be happy for this approach, albeit its going to be less trivial than what we have now (I think we may need to use dependant types to get this right)