Ahh I didn't read correctly your comment :) I thought you where saying "add
a handled method to the event handler" not the event... my fault.
The event can be immutable, lets keep it this way. Returning true/false
looks like a common pattern to indicate success/failure or
hanlded/unhandled. But if we were to drop this error handling system I'd be
fine too...not sure if people do really need it.
2015-04-29 23:03 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
> 2015-04-29 22:13 GMT+02:00 Eugen Cepoi <cepoi.eugen_at_gmail.com>:
> >
> >
> > 2015-04-29 21:56 GMT+02:00 Romain MB <rmannibucau_at_tomitribe.com>:
> >>
> >> Hi Martin,
> >>
> >> here are my feedbacks on the code:
> >>
> >> 1. severities should be an enum (I think it will not be open)
> >> 2. JsonbEventLocator more or less exists in JSON-P,any way to merge
> >> them? (ie have a json-common api -
> >>
> https://docs.oracle.com/javaee/7/api/javax/json/stream/JsonLocation.html).
> >
> >
> > I would prefer to not see a json-common :)
> > If jsonb was built on top of jsonp we wouldn't have this dilemma.
> >
> >>
> >> Plus I wouldn't use Optional here, it will mainly be used for logging
> >> and optional API is just too verbose for the most common cases IMO.
> >> Finally does the URI really means anything? We eat streams/readers so
> >> I wouldnt see URI as so important (in particularly since on write side
> >> it doesnt mean anything most of the time for the mapper framework).
> >
> >
> > It could be an optional<XXLocationXXX> and the location just contain the
> > basic position infos. I don't suggest here to define another "Location"
> for
> > jsonb.
> >
> >>
> >> 3. on the handler signature: I'd return void and add a handled()
> >> method to the event to make the API more explicit
> >
> >
> > No I think it's better to return true/false, otherwise the impl would be
> > stateful and not thread safe.
> >
>
> the event is already stateful anyway. it doesnt change the handler
> chain so not sure i get your point
>
> >>
> >>
> >> On the overall solution I have 2 points:
> >> 1. is it really needed for 1.0? Can't we just use direct exceptions
> >> like in JSON-P? I'm very tempted to wait for users feedbacks and see
> >> if it is missing before adding it.
> >>
> >> 2. Do we already have somewhere a list of events? They should be in an
> >> enum + in TCK to ensure portability otherwise kept open it is not
> >> something portable so not something the spec should handle directly
> >> IMO (as a user I would then expect to unwrap my mapper to a vendor
> >> type to access it, not to use directly the spec).
> >>
> >>
> >>
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau
> >> http://www.tomitribe.com
> >> http://rmannibucau.wordpress.com
> >> https://github.com/rmannibucau
> >>
> >>
> >> 2015-04-29 19:53 GMT+02:00 Martin Vojtek <voytoo_at_gmail.com>:
> >> > Hi Experts,
> >> >
> >> > I have pushed changes which include proposal to customize error
> >> > handling.
> >> >
> >> > Summary of the proposal:
> >> >
> >> > Erros, warning and other issues should be (similarly to JAXB)
> propagated
> >> > as
> >> > events. It will be possible to intercept (and to ignore) these events
> by
> >> > providing custom JsonbEventHandler implementation.
> >> >
> >> > Details could be found in specification and in code (api + examples).
> >> >
> >> > MartinV
> >
> >
>