TileDefinition and Definition classes refactoring

[Antonio Petrelli <an...@gmail.com>]

Hi all!
This is (I hope) the last doubt that I have, before rolling a 2.0.4
version, and it is about the TileDefinition class.
Should it be renamed to TilesDefinition, or with a name that explains
its role of "Data Transfer Object"?
The Definition class (tiles-core) differs from TileDefinition by:
- some put methods,
- the copy constructor and the template+attribute map constructor,
- the toString method,
- the "isVisited" property.

I would like to:
- move everything from Definition to TileDefinition (with its new name
:-) ) with the exception of the copy constructor and the "isVisited"
property
- rename the "isVisited" property to "visited".
In fact the Definition is needed to resolve inheritances, and the
"isVisited" flag is necessary for this purpose, but the rest seems to
belong to TileDefinition.

Thoughts?

Thanks
Antonio

Re: TileDefinition and Definition classes refactoring

[Antonio Petrelli <an...@gmail.com>]

2007/5/11, Greg Reddin <gr...@gmail.com>:
>
> On 5/11/07, Antonio Petrelli <an...@gmail.com> wrote:
> >
> > 2007/5/10, Greg Reddin <gr...@gmail.com>:
> > Well, not
> > exactly, I have in mind to create a list of definitions to be resolved
> (in
> > DefinitionsImpl), and removing them from the list when they have been
> > resolved.
>

I'm glad you like it, though the committed implementation is EXACTLY THE
OPPOSITE of what I wrote :-) I used a set to store definition names that
have been already resolved.
In fact, when I was thinking about the first solution, I thought that there
is a class that acts as a Set and allows to get the first item of that set
(without making mySet.iterator().next()). I rejected this solution because,
with a normal Set, it would create a lot of objects that will be almost
immediately discarded.

Antonio

Re: TileDefinition and Definition classes refactoring

["David H. DeWolf" <dd...@apache.org>]

+1. . .me too.  much better.

Greg Reddin wrote:
> On 5/11/07, Antonio Petrelli <an...@gmail.com> wrote:
>>
>> 2007/5/10, Greg Reddin <gr...@gmail.com>:
>> >
>> > In fact the Definition is needed to resolve inheritances, and the
>> > > "isVisited" flag is necessary for this purpose, but the rest seems to
>> > > belong to TileDefinition.
>> >
>> >
>> > I like the idea of consolidating these two classes.
>>
>>
>>
>> I think I have a better idea: moving the "visited" flag outside. Well, 
>> not
>> exactly, I have in mind to create a list of definitions to be resolved 
>> (in
>> DefinitionsImpl), and removing them from the list when they have been
>> resolved.
>> This way we need only one class, I suppose, because there will be no more
>> differences between them.
> 
> 
> I like that idea. I never was really happy with the visited flag.
> 
> Greg
> 

Re: TileDefinition and Definition classes refactoring

[Greg Reddin <gr...@gmail.com>]

On 5/11/07, Antonio Petrelli <an...@gmail.com> wrote:
>
> 2007/5/10, Greg Reddin <gr...@gmail.com>:
> >
> > In fact the Definition is needed to resolve inheritances, and the
> > > "isVisited" flag is necessary for this purpose, but the rest seems to
> > > belong to TileDefinition.
> >
> >
> > I like the idea of consolidating these two classes.
>
>
>
> I think I have a better idea: moving the "visited" flag outside. Well, not
> exactly, I have in mind to create a list of definitions to be resolved (in
> DefinitionsImpl), and removing them from the list when they have been
> resolved.
> This way we need only one class, I suppose, because there will be no more
> differences between them.


I like that idea. I never was really happy with the visited flag.

Greg

Re: TileDefinition and Definition classes refactoring

[Antonio Petrelli <an...@gmail.com>]

2007/5/10, Greg Reddin <gr...@gmail.com>:
>
> In fact the Definition is needed to resolve inheritances, and the
> > "isVisited" flag is necessary for this purpose, but the rest seems to
> > belong to TileDefinition.
>
>
> I like the idea of consolidating these two classes.



I think I have a better idea: moving the "visited" flag outside. Well, not
exactly, I have in mind to create a list of definitions to be resolved (in
DefinitionsImpl), and removing them from the list when they have been
resolved.
This way we need only one class, I suppose, because there will be no more
differences between them.

Antonio

Re: TileDefinition and Definition classes refactoring

[Greg Reddin <gr...@gmail.com>]

On 5/10/07, Antonio Petrelli <an...@gmail.com> wrote:
>
> I would like to:
> - move everything from Definition to TileDefinition (with its new name
> :-) ) with the exception of the copy constructor and the "isVisited"
> property
> - rename the "isVisited" property to "visited".


I assume you mean getVisited() and setVisited() instead of isVisited() and
setVisited()? As per the JavaBeans spec I think either is acceptable, so I'm
fine with it either way. I prefer the isVisited form for boolean properties
because that's the way NetBeans generates it for me.

In fact the Definition is needed to resolve inheritances, and the
> "isVisited" flag is necessary for this purpose, but the rest seems to
> belong to TileDefinition.


I like the idea of consolidating these two classes.

Greg