4:57 a.m.
+1 for option #1. I never likes the fact that you could add arbitrary nodes (even though
I have seen code doing it).
If we not go for #1, I prefer #2 - null from getElementDescriptor(). I don't see the
point intruding another virtual descriptor
where not all methods are clearly defined or need to be defined with constructed values.
In this case I prefer to return null
directly which of course means that there is one more null check if you are going to use
this API. On the other hand, I don't
see this API used much by actual end users. It is probably more important for integrators
of BV. In this case an additional null
check is acceptable imo.
--Hardy
On 9 Jan 2013, at 3:01 PM, Sebastian Thomschke <sebastian.thomschke(a)web.de> wrote:
I agree with Gunnar. I'm for option #1 as well.
seb
On 09.02.2013 12:12, Gunnar Morling wrote:
> My personal preference would be #1 as well.
>
> The docs don't say that non-existent nodes are allowed, so IMO it's ok to
clarify now that it is not allowed. If code relied on adding non-existent nodes, it relied
on behavior not guaranteed explicitly by the spec.
>
> --Gunnar
>
>
>
> 2013/2/8 Emmanuel Bernard <emmanuel(a)hibernate.org>
> I had the same reserve as you but then reading the JavaDoc made me
> realize that the doc mentions properties so it is technically not as
> vague as I initially thought.
>
> Emmanuel
>
> On Fri 2013-02-08 11:48, Matt Benson wrote:
> > Personally my reason for eschewing option #1 is the possibility of breaking
> > existing code.
> >
> > Matt
> >
> >
> > On Fri, Feb 8, 2013 at 11:22 AM, Emmanuel Bernard
<emmanuel(a)hibernate.org>wrote:
> >
> > > At this time of day I do tend to like option #1 (ie making virtual nodes
> > > illegal). Otherwise my preferred option is #4
> > > (VirtualElementDescriptor).
> > >
> > > Out of curiosity for people not preferring option #1. What use case do
> > > you see in having virtual nodes?
> > >
> > > Emmanuel
> > >
> > > On Fri 2013-02-08 11:08, Matt Benson wrote:
> > > > Hmm, a third option for the elementClass of such a descriptor might
be
> > > > Void.class.
> > > >
> > > > I prefer option #3, followed by #4.
> > > >
> > > > Matt
> > > >
> > > >
> > > > On Fri, Feb 8, 2013 at 5:17 AM, Gunnar Morling
<gunnar(a)hibernate.org>
> > > wrote:
> > > >
> > > > > Hi experts,
> > > > >
> > > > > One of the remaining issues on our way to 1.1 is BVAL-336 [1],
which is
> > > > > about what to return from Node#getElementDescriptor() for nodes
added
> > > by
> > > > > the user via the ConstraintViolationBuilder API.
> > > > >
> > > > > Note that as per the spec only sub paths can be created by the
user,
> > > i.e.
> > > > > user added nodes always represent properties but e.g. no methods
or
> > > > > parameters. Based on that, it seems right to return the correct
> > > > > PropertyDescriptor in case a user adds a node for an existing
property.
> > > > >
> > > > > Things get tricky though when nodes are added for non-existent
> > > properties.
> > > > > Emmanuel, Hardy and I identified the following options to address
this
> > > > > situation:
> > > > >
> > > > > #1 Disallow adding non-existent nodes
> > > > >
> > > > > In case a user adds a node but no property with that names exist,
an
> > > > > exception is thrown. The problem is elegantly avoided that way,
but we
> > > > > might break some 1.0 user code (currently the spec is not really
clear
> > > > > whether added nodes must exist or not).
> > > > >
> > > > > #2 return null from getElementDescriptor()
> > > > >
> > > > > When invoking getElementDescriptor() on a Node representing a
> > > non-existent
> > > > > property, null could be returned. This seems consistent (there is
no
> > > > > element), but causes additional null checking when traversing a
path.
> > > > >
> > > > > #3 return a PropertyDescriptor from getElementDescriptor()
> > > > >
> > > > > Since all user-added nodes represent properties, a
PropertyDescriptor
> > > > > could be returned. hasConstraints() would return false,
> > > > > getConstraintDescriptors() the empty set etc. This would allow
to
> > > handle
> > > > > all nodes and their descriptors uniformly when traversing a
path.
> > > Question
> > > > > is what to return from PropertyDescriptor#getElementClass().
Options
> > > are to
> > > > > return null (signaling that the property is non-existent) or
> > > Object.class.
> > > > >
> > > > > #4 return a VirtualDescriptor from getElementDescriptor()
> > > > >
> > > > > We could create a new Kind, VIRTUAL, and an accompanying type
> > > > > VirtualDescriptor and return an instance of this. Behavior of
the
> > > methods
> > > > > on the descriptor would be basically the same as in #3;
getKind()
> > > returning
> > > > > Kind.VIRTUAL would allow for a very explicit checking whether the
node
> > > > > exists or not.
> > > > >
> > > > > Any feedback on the options (ideally with arguments pro/con) or
other
> > > > > alternatives are highly welcome. As life goes, Emmanuel's,
Hardy's and
> > > my
> > > > > preferences are equally distributed between these options :)
> > > > >
> > > > > Thanks,
> > > > >
> > > > > --Gunnar
> > > > >
> > > > > [1] https://hibernate.onjira.com/browse/BVAL-336
> > > > >
> > > > >
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev