https://kotlinlang.org logo
Channels
Powered by
#klaxon
Title
# klaxon
c

cedric

10/10/2018, 7:12 PM
Try invalid JSON? @arve
s

Shawn

10/10/2018, 7:18 PM
in my experience, invalid JSON yields a 
KlaxonException
with the specific parsing error
e.g. 
com.beust.klaxon.KlaxonException: Expected "}", a value or ",", not 'EOF' at line 1
a

arve

10/10/2018, 7:18 PM
Yeah
c

cedric

10/10/2018, 7:19 PM
@Shawn I was actually thinking just that shortly after I wrote my incorrect answer 🙂
a

arve

10/10/2018, 7:20 PM
this is the test that i'm mutating to try and trigger a null result
Copy code
data class Thing(val foo: String)
assert(Klaxon().parse<Thing>("]") == null)
I'm trying to a) Understand why the return type of 
parse
is 
T?
insted of 
T
and b) create a test for my helper methods' null handling 🙂
k

keturn

10/10/2018, 7:27 PM
yeah this came up a little while back and nobody had an answer for why that is nullable then either: https://kotlinlang.slack.com/archives/C90AVCDQU/p1535335916000200
k

karelpeeters

10/10/2018, 7:34 PM
It can't be 
null
, since at some point the 
parse
functions call 
parseFromJsonObject
which calls 
fromJsonObject
, that returns 
Any
and the result is cast to 
T?
.
a

arve

10/10/2018, 7:42 PM
Yeah, i just found that part. Is there a reason for this cast that I'm not seeing? Seems to me like it would be an improvement in ergonomics to drop the cast and update return types in the call chain to 
T
.. Would a PR for this be welcome? @cedric
c

cedric

10/10/2018, 7:43 PM
Don’t bother, I’m fixing it right now
Removing the nullable type
🙏 2
💯 1
Copy code
* @return a JsonObject or JsonArray
Really wish this didn’t have to be a comment 🙂
Done.
Guess I could introduce a breaking change and return a 
Either<JsonObject, JsonArray>
instead
k

karelpeeters

10/10/2018, 8:47 PM
Can't you make both of them extend a sealed parent class and return that?
That shouldn't be a breaking change.
c

cedric

10/10/2018, 8:48 PM
That’s another option, yes
It would be a breaking change for the JAckson parser plug-in, but well, it’s part of the repo so not really external
but it would break other plug-ins (don’t think there is any at the moment though)
k

karelpeeters

10/10/2018, 8:50 PM
Not familiar with that plugin (or the concept of plugins at all), how would it break?
c

cedric

10/10/2018, 8:51 PM
It’s recent: you can now plug your own parser to use instead of Klaxon’s. There is one using Jackson now, which is faster if you are parsing big documents
It would break because it’s overriding 
parse(): Any?
Then again, I just broke it by removing nullability 🙂
I might as well create a base class 
JsonObjectOrArray
I guess
k

karelpeeters

10/10/2018, 8:53 PM
Oh right there was a discussion about performance there, didn't know the result was called "plugins".
That name though 😶
c

cedric

10/10/2018, 8:54 PM
Which name don’t you like, 
JsonObjectOrArray
? Open to suggestions
BaseJson
?
s

Shawn

10/10/2018, 8:54 PM
BaSON
, surely 😛
k

karelpeeters

10/10/2018, 8:59 PM
JsonBase
then for consistency.
c

cedric

10/10/2018, 8:59 PM
Sounds good.
k

karelpeeters

10/10/2018, 8:59 PM
This definitely going into breaking territory but why are there functions that can return both? Is there ever a situation in which the caller doesn't know what it should be in advance?
c

cedric

10/10/2018, 8:59 PM
Well I already have one 🙂
😢 1
It can happen yes, but this is for the low level API
the object binding API has 
parse()
and 
parseArray()
k

karelpeeters

10/10/2018, 9:00 PM
Hmm, feels weird having that information and throwing it away.
c

cedric

10/10/2018, 9:00 PM
A downcast will still be necessary but narrowing the potential types to two is better than just having 
Any
all the object binding API does is cast it for you
Well, 
JsonObject
and 
JsonArray
already extend 
JSonBase
, so we’re done here 🙂
k

karelpeeters

10/10/2018, 9:02 PM
sealed interface
c

cedric

10/10/2018, 9:03 PM
They’re in different compilation units but I could fix that too obviously
Powered by