I filed a few bugs. Ralf, thank you for your analysis. Regards,
Wim On Mon, Jul 2, 2012 at 2:47 PM, Hallvard Trætteberg <[email protected]> wrote: > Ralf, > > In general I agree with your suggestions and have already been considering > many of the changes. I've hesitated since I'm not the original author, but > just helped to move it to Nebula. However, the original author (Stepan > Rutz) has not yet committer status and has been very silent lately, so I > guess I'm entitled to decide on these issues myself. > > Comments/answers are inlined. > > > On 02.07.12 14.21, Ralf Sternberg wrote: > >> >> A few things I notice about the API, they are not particularly RAP >> specific, but let me point them out anyway: >> >> * There are a lot of public methods in the GeoMap class, some of them >> may not be really needed for using the widget itself. For example, the >> developer who uses a GeoMap widget will care about the location >> displayed and the zoom level, but probably not about how this is >> achieved. Not exposing internals like the number and size of tiles, or >> maybe make them part of another class (TileServer?), would make the >> widget easier to change. Since API should be forever, I think it's a >> good practice to start with a minimal API and evolve it as needed. >> > > I agree that the API could and should be minimized, and only later > extended based on stated needs. The current API has not been reconsidered > or trimmed from the original contribution, although I have done some > refactoring. Besides a good design principle, a minimized API is easier to > document ;-) > > > * Does TileCache and AsyncImage need to be public? >> > > I think they were made public to support some statistics shown in an > example. They don't really need to be public. > > > * What about extracting the public inner classes to separate files? >> > > Possibly. > > > * What about an SWT-style listener interface like the MapListener in >> the Google maps widget for RAP [1]? >> > > Yes, I've also found the use of PropertyChangeEvents strange in an SWT > world. I'm not sure about addressResolved, but the other two (centerChanged > and zoomChanged) clearly make sense. > > > * I believe that SWT widgets should not use logging and not involve a >> dependency to a logging framework. >> > > Could perhaps issue errors/warnings through the MapListener interface, so > the client can decide. Wim, is there a Nebula policy on this? > > Ralf, it would be nice if you could copy your comments into one or more > bugzilla enhancement requests, so I can keep track of and address them. > > Hallvard > > ______________________________**_________________ > nebula-dev mailing list > [email protected] > https://dev.eclipse.org/**mailman/listinfo/nebula-dev<https://dev.eclipse.org/mailman/listinfo/nebula-dev> >
_______________________________________________ nebula-dev mailing list [email protected] https://dev.eclipse.org/mailman/listinfo/nebula-dev
