[
http://jira.magnolia-cms.com/browse/MGNLUI-606?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Espen Jervidalo updated MGNLUI-606:
-----------------------------------
Description:
It's an abstract mess in general grown over time. This could be simplified a
lot by simple refactorings, documentation and probably get rid of some classes.
all those methods in there take a node or an Id to perform actions, but the the
classes are wrapping/adapting a single property or node, so you could
theoretically create an adapter and pass a not relating node into it. Why pass
those references around?
---
info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter
is using the same lists as the node adapters to keep track of changed
properties. The properties beeing used inside a property are:
- ModelConstants.JCR_NAME
- JcrPropertyAdapter#VALUE_PROPERTY
- JcrPropertyAdapter#TYPE_PROPERTY
It's very confusing. There are properties inside properties. They override the
same abstract functions, but do different things.
The constants are locally stored, but also using ModelConstants.
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#changedProperties
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#removedProperties
---
methods doing almost the same but not really, confusing:
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#updateProperty
info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter#updateProperty
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getItemProperty
info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter#updateProperty
---
property and nodes using getJcrItem, which is not a good idea from a
performance perpective. And we do know on which type of item we are working on,
so change it to getProperty and getNode. We end up casting every time we get a
property or node from the repo.
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#getJcrItem
---
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getNode
contains these lines of code:
// get Node from repository
node = (Node) getJcrItem();
50 lines earlier, we have a method:
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getNodeFromRepository()
{
return (Node) getJcrItem();
---
DefaultProperty is used for creating an actual default property based on some
default value but mostly it's used to create properties with non-default
values. The term default may be misleading. same goes for DefaultPropertyUtil.
As we're using generics in DefaultProperty now, maybe introducing generics in
Adapters would make sense?
---
DefaultPropertyUtil vs info.magnolia.jcr.util.PropertyUtil
was:
It's an abstract mess in general grown over time. This could be simplified a
lot by simple refactorings, documentation and probably get rid of some classes.
all those methods in there take a node or an Id to perform actions, but the the
classes are wrapping/adapting a single property or node, so you could
theoretically create an adapter and pass a not relating node into it. Why pass
those references around?
---
info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter
is using the same lists as the node adapters to keep track of changed
properties. The properties beeing used inside a property are:
- ModelConstants.JCR_NAME
- JcrPropertyAdapter#VALUE_PROPERTY
- JcrPropertyAdapter#TYPE_PROPERTY
It's very confusing. There are properties inside properties. They override the
same abstract functions, but do different things.
The constants are locally stored, but also using ModelConstants.
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#changedProperties
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#removedProperties
---
methods doing almost the same but not really, confusing:
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#updateProperty
info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter#updateProperty
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getItemProperty
info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter#updateProperty
---
property and nodes using getJcrItem, which is not a good idea from a
performance perpective. And we do know on which type of item we are working on,
so change it to getProperty and getNode. We end up casting every time we get a
property or node from the repo.
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#getJcrItem
---
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getNode
contains these lines of code:
// get Node from repository
node = (Node) getJcrItem();
50 lines earlier, we have a method:
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getNodeFromRepository()
{
return (Node) getJcrItem();
---
DefaultProperty is used for creating an actual default property based on some
default value but mostly it's used to create properties with non-default
values. The term default may be misleading. same goes for DefaultPropertyUtil.
---
DefaultPropertyUtil vs info.magnolia.jcr.util.PropertyUtil
> info.magnolia.ui.vaadin.integration.jcr whole package need review
> -----------------------------------------------------------------
>
> Key: MGNLUI-606
> URL: http://jira.magnolia-cms.com/browse/MGNLUI-606
> Project: Magnolia UI
> Issue Type: Task
> Security Level: Public
> Affects Versions: 5.0 Alpha2 s011
> Reporter: Espen Jervidalo
> Fix For: 5.0 Alpha2 s012
>
>
> It's an abstract mess in general grown over time. This could be simplified a
> lot by simple refactorings, documentation and probably get rid of some
> classes.
> all those methods in there take a node or an Id to perform actions, but the
> the classes are wrapping/adapting a single property or node, so you could
> theoretically create an adapter and pass a not relating node into it. Why
> pass those references around?
> ---
> info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter
> is using the same lists as the node adapters to keep track of changed
> properties. The properties beeing used inside a property are:
> - ModelConstants.JCR_NAME
> - JcrPropertyAdapter#VALUE_PROPERTY
> - JcrPropertyAdapter#TYPE_PROPERTY
> It's very confusing. There are properties inside properties. They override
> the same abstract functions, but do different things.
> The constants are locally stored, but also using ModelConstants.
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#changedProperties
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#removedProperties
> ---
> methods doing almost the same but not really, confusing:
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#updateProperty
> info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter#updateProperty
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getItemProperty
> info.magnolia.ui.vaadin.integration.jcr.JcrPropertyAdapter#updateProperty
> ---
> property and nodes using getJcrItem, which is not a good idea from a
> performance perpective. And we do know on which type of item we are working
> on, so change it to getProperty and getNode. We end up casting every time we
> get a property or node from the repo.
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#getJcrItem
> ---
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getNode
> contains these lines of code:
> // get Node from repository
> node = (Node) getJcrItem();
> 50 lines earlier, we have a method:
> info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getNodeFromRepository()
> {
> return (Node) getJcrItem();
> ---
> DefaultProperty is used for creating an actual default property based on some
> default value but mostly it's used to create properties with non-default
> values. The term default may be misleading. same goes for DefaultPropertyUtil.
> As we're using generics in DefaultProperty now, maybe introducing generics in
> Adapters would make sense?
> ---
> DefaultPropertyUtil vs info.magnolia.jcr.util.PropertyUtil
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.magnolia-cms.com/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
----------------------------------------------------------------
For list details, see: http://www.magnolia-cms.com/community/mailing-lists.html
Alternatively, use our forums: http://forum.magnolia-cms.com/
To unsubscribe, E-mail to: <dev-list-unsubscr...@magnolia-cms.com>
----------------------------------------------------------------
