Implement visitFloat32, visitFloat64String for Byte/Short/Int/Long/Float/Double/Char

[htmldoug]

Fixes #354.

[lihaoyi]

@htmldoug looks good, feel free to merge

[lihaoyi]

@htmldoug could we add a unit test too? just to make sure this is fixing what we it should be fixing

[htmldoug]

Sure. Tests found visitFloat64String missing, too. JsVisitor isn't on the classpath here, so I just copy-pasted. If that's okay with you, it's merge-ready.

[lihaoyi]

looks good to me

[lihaoyi]

Not sure if the copypaste code can easily be moved into some private helper function, but if it can that would help DRY things up a bit

[htmldoug]

Refactored out a NumericReader. protected is the right visibility, I guess?

[lihaoyi]

Seems good enough. This repo isn't particularly strict about visibility anyway. Feel free to merge

[lihaoyi]

TBH I'd have put the logic into a static helper function somewhere that the various readers call from their override of visitFloat64String, rather than a trait that the various readers extend. That seems like it would be a lot simpler, reduce the depth of the inheritance hierarchies, and make the logic easier to reuse if someone else wants to call it in odd places.

We do a lot of fancy trait cake stuff in the repo but I like static functions where we can get away with then

[htmldoug]

I agree in principle, although it's a little tricky since they need to delegate to other instance methods.

Would either of these approaches be better?

def helper(s: String): (String, Int, Int) = ???

// each Reader:
def visitFloat64String(s: String, index: Int): T = {
  val (s, decIndex, expIndex) = helper(s)
  visitFloat64StringParts(s, decIndex, expIndex, index)
}

def helper[T](s: String, visitFloat64StringParts: (String, Int, Int) => T): T = ???

// each Reader:
def visitFloat64String(s: String, index: Int) = helper(s, visitFloat64StringParts(_, _, _, index))

[lihaoyi]

Oh if a static helper is tricky lets just go with the trait then