Code Review: removing static cache from MapPane

0 views
Skip to first unread message

Eric Ayers

unread,
Mar 17, 2008, 2:13:06 PM3/17/08
to Google Web Toolkit Contributors
Miguel,

I would like for you to review this change to the gwt-google-apis library.

The MapPane class incorrectly cached instances of created map panes in a static variable.  This was inappropriate as a second call with a different instance of a MapWidget would cause the wrong DIV element to be returned.  If we want to add caching of MapPane instances, we should probably do it in MapWidget.

This change is a breaking change in that the constants are changed from being static final members of MapPane to a separate enum named MapPaneType.  The fix is to add an import of com.google.gwt.maps.client.MapPaneType and to prepent the XXX_PANE constant value with the label MapPaneType (the name of the enum)

As a part of this change, I added initialization of the MapPaneType directly from the Maps API JavaScript constants instead of hard coding them as integers in the MapPane.java by following the convention of creating an enum using JSOpaque types I observed in the ajaxsearch API.  As another benefit, in the end, this greatly reduces the line number count in MapPane.java.

$ diffstat ~/Patches/galgwt-mappane-r158.patch
 google-apis/src/com/google/gwt/maps/client/MapPane.java                        |   89 +---------
 google-apis/src/com/google/gwt/maps/client/MapPaneType.java                    |   41 ++++
 google-apis/src/com/google/gwt/maps/client/MapWidget.java                      |    1
 samples/maps/src/com/google/gwt/maps/sample/maps/client/CustomOverlayDemo.java |    3
 4 files changed, 55 insertions(+), 79 deletions(-)

--
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

Eric Ayers

unread,
Mar 17, 2008, 3:54:26 PM3/17/08
to Google Web Toolkit Contributors, Miguel Méndez
Attached is the missing .patch file:
galgwt-mappane-r158.patch

Eric Ayers

unread,
Mar 18, 2008, 9:46:17 AM3/18/08
to Google Web Toolkit Contributors, Miguel Méndez
Please ignore this thread.  The patch is now wound into the patch attached to the discussion with the subject:

Code Review: gwt-google-apis some new Javadoc, constants cleanups, method name refactoring (issue65)
Reply all
Reply to author
Forward
0 new messages