You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@pivot.apache.org by calathus <ca...@gmail.com> on 2010/12/23 04:27:31 UTC
pivot Collection overrides equals, hashCode, bug?
Hi,
org.apache.pivot.collections.ArrayList, HashMap, HashSet are overriding
equals and hashCode.
When a new element is added(or removed), the value of hashCode will be
changed.
This is problematic for using the instance(ArrayList) in a dictionary like
java.util.HashMap as the key value.
Essentially we cannot use it for key value.
I wonder this is intentional or bug.
If you need to compare the value based comparison, it might be better not to
override equal but introduce another method(e.g, equivalent).
I had some bug related to this. It will create some surprise for the user
and it will not be easy to find the cause of the problem.
I think if the collection is immutable, the current code would make sense
though.
--
Cheers,
calathus
Re: pivot Collection overrides equals, hashCode, bug?
Posted by Greg Brown <gk...@verizon.net>.
>> Although BXML based declarative style makes the GUI specification shorter,
>> when I started using Pivot, I faced a lots of runtime error, and it was
>> quite difficult to debug, I couldn't see any clue to debug. This
>> heterogeneous approach mixing, XML, Javascript, etc reminds me old JSP
>> style approach.
IMO, there is a big difference between JSP and BXML. BXML is a shortcut for instantiating class hierarchies. You can optionally script it, but it is nowhere near as unstructured as JSP, which let you inject script or other content pretty much anywhere. It is much closer conceptually to the markup languages used by Flex and Silverlight (MXML and XAML, respectively).
Your point about debugging is valid, but it only applies if you are using script code. If your BXML file is backed by a class that implements Bindable, you can wire up your event listeners in Java and step through it like any other Java code.
Re: pivot Collection overrides equals, hashCode, bug?
Posted by Sandro Martini <sa...@gmail.com>.
Hi Calatus,
again many thanks to you for sharing this with us :-)
But to reuse some part of code we have some constraints from Apache, and
contributed code must be attached to a JIRA ticket (with the grant enabled
for inclusion in asf work) ... otherwise we could NOT use it. For now,
thanks for having included some details here.
In any case, we have to see what Greg and other Developers say, but I think
that a feature like this could really be useful, at least in a way where a
Pivot newbie need to better understand some of the internals of BXML, and in
my opinion it's always a good thing.
> Although BXML based declarative style makes the GUI specification shorter,
> when I started using Pivot, I faced a lots of runtime error, and it was
> quite difficult to debug, I couldn't see any clue to debug. This
> heterogeneous approach mixing, XML, Javascript, etc reminds me old JSP
> style approach.
I agree with you, because for some things I'm in the same situation :-) ...
So my proposal for this (but only as a starting point for discussions):
https://issues.apache.org/jira/browse/PIVOT-578
Then, like in JSP I proposed some flag to forbid the inclusion in BXML of
scripting logic ... but this probably is not so important here (there is a
discussion on this).
And us you, I think that more checks at compile time (by Java/Groovy/Scala
Builders, annotations, etc) are always a good thing, for everyone, at least
as alternative options.
> By the way, this version was based on the idea to use AOP like
> interception for Pivot objects. This approach is quite general, but use a
> lots of variables.
> And after I recognized the pattern, I think this code should not be used
> for this BXML to Java converter.
Personally I don't like too much AOP interceptors, they are a powerful
concept, but to me seems to lose control over things, and to have a too
complex part running under ... but it's a question of taste.
> Now I have almost completed a new version which generates codes with
> suggested anonymous class based coding style. I will commit this version
> later in github after I fixed some issue.
Very good.
> But if you consider this should be committed to a storage related to
> apache project, I would commit this to appropriate place for that.
Ok, but for committing to our main repository you should be a Pivot
Developer, so for the moment probably the best thing could be to enable you
on our "secondary" pivot-related projects (if you are interested, and there
aren't objections ... but I don't think there will be).
And on this, we are moving some of these projects (but I can't tell you more
details at the moment), so if you want I can grant you to write in my
"temporary" projects, like this:
http://code.google.com/p/pivot-stuff/
the only requirement is to create inside a dedicated project, and for
example (if useful) use Pivot libraries from projects already inside.
But I understand that this could be not the best workflow for you at the
moment, so don't worry, proceed as you prefer: tell me if you want some
grant, or tell us if/when you will have news (even on your projects on
GITHub).
> Anyway I think this requires more testing and review.
No problem, we are here even for this :-) , the only problem is to find time
...
> Actually I found one problem for using this approach for current Pivot
> source code.
> In order to use this coding style, the Pivot API classes should not be
> final.
> Although most of them are not final, but I found TableView.Column is
> defined as final class.
> (I locally changed this to none final class to avoid this restriction. Are
> there any good reason to make this final?)
This is my fear, see if something in Pivot prevents someone to extend
features.
On this Greg can tell you better if it's only because we are here in an
un-covered use case, or if there is a real need to have this final.
Can you give some more info on limitations you found (please maybe starting
a new thread on Dev mailing list, or forwarding this) ?
Comments ?
Note:
at the moment I'm using Nabble, and from here it's not possible to forward
this thread to the Dev Mailing List, is someone is able to do it in my place
.. thanks.
Bye,
Sandro
--
View this message in context: http://apache-pivot-users.399431.n3.nabble.com/pivot-Collection-overrides-equals-hashCode-bug-tp2134634p2162408.html
Sent from the Apache Pivot - Users mailing list archive at Nabble.com.
Re: pivot Collection overrides equals, hashCode, bug?
Posted by calathus <ca...@gmail.com>.
Sandro,
Thanks for a nice feedback.
I committed BXML2JavaConver to github already.
So you can download the source code from there.
(
In case, if you are not familiar with git, following is the instruction to
download the source code from github.
1) install git from http://git-scm.com/
2) type:
git close git://github.com/calathus/BXML-to-Java-converter.git
3) this create a folder under directory where the command is executed.
)
By the way, this version was based on the idea to use AOP like interception
for Pivot objects. This approach is quite general, but use a lots of
variables.
And after I recognized the pattern, I think this code should not be used for
this BXML to Java converter.
Now I have almost completed a new version which generates codes with
suggested anonymous class based coding style.
I will commit this version later in github after I fixed some issue.
But if you consider this should be committed to a storage related to apache
project, I would commit this to appropriate place for that.
Anyway I think this requires more testing and review.
Actually I found one problem for using this approach for current Pivot
source code. In order to use this coding style, the Pivot API classes should
not be final. Although most of them are not final, but I found
TableView.Column is defined as final class.(I locally changed this to none
final class to avoid this restriction. Are there any good reason to make
this final?)
-----
By the way, this kind coding style is independent form BXML2Java converter.
We can use this style when we want to write some Pivot application in Java
directly.
That was actually my original goal.
Although BXML based declarative style makes the GUI specification shorter,
when I started using Pivot, I faced a lots of runtime error, and it was
quite difficult to debug, I couldn't see any clue to debug. This
heterogeneous approach mixing, XML, Javascript, etc reminds me old JSP style
approach.
I think, the simplicity of Pivot to use Java for browser and desktop is very
important aspect, but somehow the current strong dependency for (b)xml file
is reducing the power of this approach.(static type checking, generic class
library, debugability etc)
If we use expressive language(even Java), we should be able to get rid of
this kind of mixture.
BTW, I will add a sample output of new code generator with declarative(?)
style (it was generated by new BXM2Java converter from
stock_tracker_window.bxml.)
--------------------
static class ROOT extends StockTrackerWindow {{
try {
setTitle("Pivot Stock Tracker");
setMaximized(true);
setContent(new TablePane() {{
setStyles("{padding:8, horizontalSpacing:6,
verticalSpacing:6}");
// columns(): READ_ONLY_PROPERTY
getColumns().add(new TablePane.Column() {{
setWidth(1, true);
}}); // INSTANCE, name: <TablePane.Column>
getRows().add(new TablePane.Row() {{
setHeight(-1);
add(new Label() {{
setText("Pivot Stock Tracker");
setStyles("{font:{size:14, bold:true},
verticalAlignment:'center'}");
}}); // INSTANCE, name: <Label>
}}); // INSTANCE, name: <TablePane.Row>
getRows().add(new TablePane.Row() {{
setHeight(1, true);
add(new SplitPane() {{
setSplitRatio(0.4f);
// left(): WRITABLE_PROPERTY
setLeft(new Border() {{
setStyles("{color:10}");
setContent(new ScrollPane() {{
setHorizontalScrollBarPolicy(ScrollPane.ScrollBarPolicy.FILL_TO_CAPACITY);
setVerticalScrollBarPolicy(ScrollPane.ScrollBarPolicy.FILL_TO_CAPACITY);
setView(new StackPane() {{
add(new TableView() {{
CodeEmitterRuntime.register("stocksTableView", this);
setSelectMode(TableView.SelectMode.MULTI);
setStyles("{showHorizontalGridLines:false}");
// columns(): READ_ONLY_PROPERTY
getColumns().add(new
TableView.Column() {{
setName("symbol");
setHeaderData("Symbol");
setWidth(1, true);
}}); // INSTANCE, name:
<TableView.Column>
getColumns().add(new
TableView.Column() {{
setName("value");
setHeaderData("Value");
setWidth(1, true);
// cellRenderer():
WRITABLE_PROPERTY
setCellRenderer(new
TableViewNumberCellRenderer() {{
setStyles("{horizontalAlignment:'right'}");
setNumberFormat("$0.00");
}}); // INSTANCE, name:
<content:TableViewNumberCellRenderer>
}}); // INSTANCE, name:
<TableView.Column>
getColumns().add(new
TableView.Column() {{
setName("change");
setHeaderData("Change");
setWidth(1, true);
// cellRenderer():
WRITABLE_PROPERTY
setCellRenderer(new
ChangeCellRenderer() {{
setStyles("{horizontalAlignment:'right'}");
setNumberFormat("+0.00;-0.00");
}}); // INSTANCE, name:
<stocktracker:ChangeCellRenderer>
}}); // INSTANCE, name:
<TableView.Column>
}}); // INSTANCE, name: <TableView>
}}); // INSTANCE, name: <StackPane>
// columnHeader(): WRITABLE_PROPERTY
setColumnHeader(new TableViewHeader() {{
setTableView((TableView)CodeEmitterRuntime.getNodeValue("stocksTableView"));
setSortMode(TableViewHeader.SortMode.SINGLE_COLUMN);
}}); // INSTANCE, name: <TableViewHeader>
}}); // INSTANCE, name: <ScrollPane>
}}); // INSTANCE, name: <Border>
// right(): WRITABLE_PROPERTY
setRight(new Border() {{
setStyles("{padding:6, color:10}");
add(new BoxPane() {{
CodeEmitterRuntime.register("detailPane", this);
setOrientation(Orientation.VERTICAL);
setStyles("{fill:true}");
add(new Label() {{
setTextKey("companyName");
setStyles("{font:{size:12,
bold:true}}");
}}); // INSTANCE, name: <Label>
add(new Separator() {{
}}); // INSTANCE, name: <Separator>
add(new Form() {{
setStyles("{padding:0, fill:true,
showFlagIcons:false, showFlagHighlight:false,
leftAlignLabels:true}");
getSections().add(new Form.Section()
{{
final ValueMapping
valueMapping_0 = (new ValueMapping() {{
CodeEmitterRuntime.register("valueMapping", this);
}}); // INSTANCE, name:
<stocktracker:ValueMapping>
final ChangeMapping
changeMapping_1 = (new ChangeMapping() {{
CodeEmitterRuntime.register("changeMapping", this);
}}); // INSTANCE, name:
<stocktracker:ChangeMapping>
final VolumeMapping
volumeMapping_2 = (new VolumeMapping() {{
CodeEmitterRuntime.register("volumeMapping", this);
}}); // INSTANCE, name:
<stocktracker:VolumeMapping>
add(new Label() {{
CodeEmitterRuntime.register("valueLabel", this);
setTextKey("value");
setTextBindMapping(valueMapping_0);
setStyles("{horizontalAlignment:'right'}");
Form.setLabel(this,
"Value");
}}); // INSTANCE, name: <Label>
add(new Label() {{
CodeEmitterRuntime.register("changeLabel", this);
setTextKey("change");
setTextBindMapping(changeMapping_1);
setStyles("{horizontalAlignment:'right'}");
Form.setLabel(this,
"Change");
}}); // INSTANCE, name: <Label>
add(new Label() {{
CodeEmitterRuntime.register("openingValueLabel", this);
setTextKey("openingValue");
setTextBindMapping(valueMapping_0);
setStyles("{horizontalAlignment:'right'}");
Form.setLabel(this, "Open");
}}); // INSTANCE, name: <Label>
add(new Label() {{
CodeEmitterRuntime.register("highValueLabel", this);
setTextKey("highValue");
setTextBindMapping(valueMapping_0);
setStyles("{horizontalAlignment:'right'}");
Form.setLabel(this, "High");
}}); // INSTANCE, name: <Label>
add(new Label() {{
CodeEmitterRuntime.register("lowValueLabel", this);
setTextKey("lowValue");
setTextBindMapping(valueMapping_0);
setStyles("{horizontalAlignment:'right'}");
Form.setLabel(this, "Low");
}}); // INSTANCE, name: <Label>
add(new Label() {{
CodeEmitterRuntime.register("volumeLabel", this);
setTextKey("volume");
setTextBindMapping(volumeMapping_2);
setStyles("{horizontalAlignment:'right'}");
Form.setLabel(this,
"Volume");
}}); // INSTANCE, name: <Label>
}}); // INSTANCE, name:
<Form.Section>
}}); // INSTANCE, name: <Form>
}}); // INSTANCE, name: <BoxPane>
}}); // INSTANCE, name: <Border>
}}); // INSTANCE, name: <SplitPane>
}}); // INSTANCE, name: <TablePane.Row>
getRows().add(new TablePane.Row() {{
setHeight(-1);
add(new BoxPane() {{
setStyles("{horizontalAlignment:'left',
verticalAlignment:'center'}");
add(new Label() {{
setText("Symbol");
setStyles("{font:{bold:true}}");
}}); // INSTANCE, name: <Label>
add(new TextInput() {{
CodeEmitterRuntime.register("symbolTextInput",
this);
setTextSize(10);
setMaximumLength(8);
}}); // INSTANCE, name: <TextInput>
add(new LinkButton() {{
CodeEmitterRuntime.register("addSymbolButton",
this);
setEnabled(false);
setTooltipText("Add symbol");
setButtonData(new ButtonData() {{
setIcon(new
URL("file:/share/workspace/pivot/tutorials/src/org/apache/pivot/tutorials/stocktracker/add.png"));
}}); // INSTANCE, name: <content:ButtonData>
}}); // INSTANCE, name: <LinkButton>
add(new LinkButton() {{
CodeEmitterRuntime.register("removeSymbolsButton", this);
setEnabled(false);
setTooltipText("Remove selected symbols");
setButtonData(new ButtonData() {{
setIcon(new
URL("file:/share/workspace/pivot/tutorials/src/org/apache/pivot/tutorials/stocktracker/delete.png"));
}}); // INSTANCE, name: <content:ButtonData>
}}); // INSTANCE, name: <LinkButton>
}}); // INSTANCE, name: <BoxPane>
}}); // INSTANCE, name: <TablePane.Row>
getRows().add(new TablePane.Row() {{
setHeight(-1);
add(new TablePane() {{
// columns(): READ_ONLY_PROPERTY
getColumns().add(new TablePane.Column() {{
setWidth(1, true);
}}); // INSTANCE, name: <TablePane.Column>
getColumns().add(new TablePane.Column() {{
setWidth(-1);
}}); // INSTANCE, name: <TablePane.Column>
getRows().add(new TablePane.Row() {{
add(new BoxPane() {{
add(new Label() {{
setText("Last Update");
}}); // INSTANCE, name: <Label>
add(new Label() {{
CodeEmitterRuntime.register("lastUpdateLabel", this);
}}); // INSTANCE, name: <Label>
}}); // INSTANCE, name: <BoxPane>
add(new BoxPane() {{
setStyles("{horizontalAlignment:'right'}");
add(new Label() {{
setText("Data provided by");
}}); // INSTANCE, name: <Label>
add(new LinkButton() {{
CodeEmitterRuntime.register("yahooFinanceButton", this);
setButtonData("Yahoo! Finance");
}}); // INSTANCE, name: <LinkButton>
}}); // INSTANCE, name: <BoxPane>
}}); // INSTANCE, name: <TablePane.Row>
}}); // INSTANCE, name: <TablePane>
}}); // INSTANCE, name: <TablePane.Row>
}}); // INSTANCE, name: <TablePane>
CodeEmitterRuntime.bind(ROOT.this, "stocksTableView");
CodeEmitterRuntime.bind(ROOT.this, "symbolTextInput");
CodeEmitterRuntime.bind(ROOT.this, "addSymbolButton");
CodeEmitterRuntime.bind(ROOT.this, "removeSymbolsButton");
CodeEmitterRuntime.bind(ROOT.this, "detailPane");
CodeEmitterRuntime.bind(ROOT.this, "lastUpdateLabel");
CodeEmitterRuntime.bind(ROOT.this, "yahooFinanceButton");
} catch (Exception e) {
e.printStackTrace();
throw new RuntimeException(e);
}
}}
On Tue, Dec 28, 2010 at 3:01 PM, Sandro Martini <sa...@gmail.com>wrote:
> Hi all,
> (last time I answered to the other thread, so I repeat here)
>
> @calathus:
> great work !! I think this idea is much interesting from many points of
> view.
> Are you interested in sharing with us even the first code you produced
> (but adding it in a jira ticket, otherwise we will not be able to use
> it, by Apache legal constraints) ?
>
> @all (developers and not only):
> what do you think on have the BXML to Java feature inside a (re-enabled)
> developer-only Tools subproject (or something similar), for the 2.1 release
> ?
> And last:
> the changes that calatus made in BXMLSerializer, maybe could be
> applied to the "standard" version of BXMLSerializer ... or better to a
> Tools-specific extension of it (if could be done) ... open a ticket
> for the 2.1 ?
>
> Bye,
> Sandro
>
--
Cheers,
calathus
Re: pivot Collection overrides equals, hashCode, bug?
Posted by Sandro Martini <sa...@gmail.com>.
Hi all,
(last time I answered to the other thread, so I repeat here)
@calathus:
great work !! I think this idea is much interesting from many points of view.
Are you interested in sharing with us even the first code you produced
(but adding it in a jira ticket, otherwise we will not be able to use
it, by Apache legal constraints) ?
@all (developers and not only):
what do you think on have the BXML to Java feature inside a (re-enabled)
developer-only Tools subproject (or something similar), for the 2.1 release
?
And last:
the changes that calatus made in BXMLSerializer, maybe could be
applied to the "standard" version of BXMLSerializer ... or better to a
Tools-specific extension of it (if could be done) ... open a ticket
for the 2.1 ?
Bye,
Sandro
Re: pivot Collection overrides equals, hashCode, bug?
Posted by calathus <ca...@gmail.com>.
Greg,
I actually end up implementing the BXML to Java converter. It is now working
for basic BXML feature(excluing Javascript, listener).
The code is available from:
https://github.com/calathus/BXML-to-Java-converter/blob/master/README
This tools was implemented with AOP like idea to intercept the actual Pivot
object instantiation/modification and generate corresponding Java source
code.
I modified the BMXLSerializer so that it will call back code emitter
interface.(So the change is minimal for the BMXLSerializer).
In order to map objects to variable names used for Java source code, I
needed to use Map. Normally the objects are ordinary Pivot class's
instances. But some internal objects are ArrayList.
So I avoided to use java.util.HashMap, and created custom environment class
using List. For this application, since it will not create big table, this
will be sufficient.
The comment/feed back for this tool is welcome.
On Thu, Dec 23, 2010 at 5:24 AM, Greg Brown <gk...@verizon.net> wrote:
> I'd expect that you would run into the same issue using the JDK
> collections. From the Javadoc for java.util.Map:
>
> "Note: great care must be exercised if mutable objects are used as map
> keys. The behavior of a map is not specified if the value of an object is
> changed in a manner that affects equals comparisons while the object is a
> key in the map."
>
> Certainly adding or removing an item from a list should affect the value of
> the equals() method, and probably hashCode() as well.
>
> What is the use case for using a list as a key?
>
> On Dec 22, 2010, at 10:27 PM, calathus wrote:
>
> > Hi,
> > org.apache.pivot.collections.ArrayList, HashMap, HashSet are overriding
> equals and hashCode.
> > When a new element is added(or removed), the value of hashCode will be
> changed.
> > This is problematic for using the instance(ArrayList) in a dictionary
> like java.util.HashMap as the key value.
> > Essentially we cannot use it for key value.
> >
> > I wonder this is intentional or bug.
> > If you need to compare the value based comparison, it might be better not
> to override equal but introduce another method(e.g, equivalent).
> >
> > I had some bug related to this. It will create some surprise for the user
> and it will not be easy to find the cause of the problem.
> > I think if the collection is immutable, the current code would make sense
> though.
> >
> > --
> > Cheers,
> > calathus
> >
> >
> >
>
>
--
Cheers,
calathus
Re: pivot Collection overrides equals, hashCode, bug?
Posted by Greg Brown <gk...@verizon.net>.
I'd expect that you would run into the same issue using the JDK collections. From the Javadoc for java.util.Map:
"Note: great care must be exercised if mutable objects are used as map keys. The behavior of a map is not specified if the value of an object is changed in a manner that affects equals comparisons while the object is a key in the map."
Certainly adding or removing an item from a list should affect the value of the equals() method, and probably hashCode() as well.
What is the use case for using a list as a key?
On Dec 22, 2010, at 10:27 PM, calathus wrote:
> Hi,
> org.apache.pivot.collections.ArrayList, HashMap, HashSet are overriding equals and hashCode.
> When a new element is added(or removed), the value of hashCode will be changed.
> This is problematic for using the instance(ArrayList) in a dictionary like java.util.HashMap as the key value.
> Essentially we cannot use it for key value.
>
> I wonder this is intentional or bug.
> If you need to compare the value based comparison, it might be better not to override equal but introduce another method(e.g, equivalent).
>
> I had some bug related to this. It will create some surprise for the user and it will not be easy to find the cause of the problem.
> I think if the collection is immutable, the current code would make sense though.
>
> --
> Cheers,
> calathus
>
>
>