[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [nebula-dev] Let's become friends
|
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
<hal@xxxxxxxxxxx> 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
