Simplify NodeImpl

[Tobias Bocanegra <to...@day.com>]

hi all,
i'm currently looking at the versioning API of  jsr283. there is a new
VersionManager introduced that provides all versioning related methods
and those on javax.jcr.Node are deprecated. i want to use this as an
occasion to (re)move the versioning methods in NodeImpl.

basically there are 2 possibilities:
a) either move them to the version manager
b) or move them to a new class that extends NodeImpl, for example
VersionableNodeImpl.

the first (a) would align more with the JCR api, the latter (b) would
be more object oriented. personally i prefer a VersionableNodeImpl
solution, since most of the operations need access to protected
methods on NodeImpl anyways. however, it also has the drawback, that
for each new instance of NodeImpl, the item manager needs to check if
the node is mix:versionable and then create a different
implementation. this check is currently relative expensive, because
the effective node type is not calculated at this point.

WDYT?

regards, toby

Re: Simplify NodeImpl

[Tobias Bocanegra <tr...@day.com>]

On Tue, Apr 14, 2009 at 9:48 AM, Alexander Klimetschek <ak...@day.com> wrote:
> On Sun, Apr 12, 2009 at 3:36 PM, Tobias Bocanegra
> <to...@day.com> wrote:
>> hi all,
>> i'm currently looking at the versioning API of  jsr283. there is a new
>> VersionManager introduced that provides all versioning related methods
>> and those on javax.jcr.Node are deprecated. i want to use this as an
>> occasion to (re)move the versioning methods in NodeImpl.
>>
>> basically there are 2 possibilities:
>> a) either move them to the version manager
>> b) or move them to a new class that extends NodeImpl, for example
>> VersionableNodeImpl.
>>
>> the first (a) would align more with the JCR api, the latter (b) would
>> be more object oriented. personally i prefer a VersionableNodeImpl
>> solution, since most of the operations need access to protected
>> methods on NodeImpl anyways. however, it also has the drawback, that
>> for each new instance of NodeImpl, the item manager needs to check if
>> the node is mix:versionable and then create a different
>> implementation. this check is currently relative expensive, because
>> the effective node type is not calculated at this point.
>
> And if one adds mix:versionable to an existing node, the NodeImpl in
> the hand of the client has to be "transformed" into a
> VersionableNodeImpl. I'd favor (b) as well, but I think it introduces
> too much problems.
right.

> What does deprecated mean for Jackrabbit anyway? Shall these methods
> continue to work as before until the next (minor) release to give
> users a chance to smoothly upgrade? Or does JSR-283 enforce something
> like an UnsupportedOperationExeception for them?
deprecated means, that they might be removed for jcr3.0.

maybe it would really make more sense to move the versioning
operations to the version manager, since most of them are workspace
operations anyways and should not use the transient space at all. in
this respect, removing them from the item instance is valid and
desirable.

regards, toby

Re: Simplify NodeImpl

[Alexander Klimetschek <ak...@day.com>]

On Sun, Apr 12, 2009 at 3:36 PM, Tobias Bocanegra
<to...@day.com> wrote:
> hi all,
> i'm currently looking at the versioning API of  jsr283. there is a new
> VersionManager introduced that provides all versioning related methods
> and those on javax.jcr.Node are deprecated. i want to use this as an
> occasion to (re)move the versioning methods in NodeImpl.
>
> basically there are 2 possibilities:
> a) either move them to the version manager
> b) or move them to a new class that extends NodeImpl, for example
> VersionableNodeImpl.
>
> the first (a) would align more with the JCR api, the latter (b) would
> be more object oriented. personally i prefer a VersionableNodeImpl
> solution, since most of the operations need access to protected
> methods on NodeImpl anyways. however, it also has the drawback, that
> for each new instance of NodeImpl, the item manager needs to check if
> the node is mix:versionable and then create a different
> implementation. this check is currently relative expensive, because
> the effective node type is not calculated at this point.

And if one adds mix:versionable to an existing node, the NodeImpl in
the hand of the client has to be "transformed" into a
VersionableNodeImpl. I'd favor (b) as well, but I think it introduces
too much problems.

What does deprecated mean for Jackrabbit anyway? Shall these methods
continue to work as before until the next (minor) release to give
users a chance to smoothly upgrade? Or does JSR-283 enforce something
like an UnsupportedOperationExeception for them?

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: Simplify NodeImpl

[Stefan Guggisberg <st...@gmail.com>]

On Tue, Apr 14, 2009 at 12:28 PM, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> On Sun, Apr 12, 2009 at 3:36 PM, Tobias Bocanegra
> <to...@day.com> wrote:
>> the first (a) would align more with the JCR api, the latter (b) would
>> be more object oriented. personally i prefer a VersionableNodeImpl
>> solution, since most of the operations need access to protected
>> methods on NodeImpl anyways.
>
> A class should only implement a clearly scoped set of functionality.
> The Node interface in JCR is much too large for that, so personally
> I'd rather move towards making NodeImpl a facade that just directs the
> flow of control over multiple more tightly scoped objects.

+1

>
> The "access to protected methods" issue is IMHO a symptom of the
> current tendency to implement so much inside the Node object. I'd
> rather solve that issue by refactoring such protected methods into
> public methods of separate objects that are only handed to those
> places that need access to that functionality. Composition over
> inheritance.

+1

cheers
stefan

>
> BR,
>
> Jukka Zitting
>

Re: Simplify NodeImpl

[Jukka Zitting <ju...@gmail.com>]

Hi,

On Sun, Apr 12, 2009 at 3:36 PM, Tobias Bocanegra
<to...@day.com> wrote:
> the first (a) would align more with the JCR api, the latter (b) would
> be more object oriented. personally i prefer a VersionableNodeImpl
> solution, since most of the operations need access to protected
> methods on NodeImpl anyways.

A class should only implement a clearly scoped set of functionality.
The Node interface in JCR is much too large for that, so personally
I'd rather move towards making NodeImpl a facade that just directs the
flow of control over multiple more tightly scoped objects.

The "access to protected methods" issue is IMHO a symptom of the
current tendency to implement so much inside the Node object. I'd
rather solve that issue by refactoring such protected methods into
public methods of separate objects that are only handed to those
places that need access to that functionality. Composition over
inheritance.

BR,

Jukka Zitting