Use of static var in MapScript mapObj.nextLabel

[Daniel Morissette]

I agree with both of you!  ;)

I mean:

- I agree it's a bad idea to keep client related state (such as last 
label read) in a static var, or even in something like a mapObj.

- I agree that we should not remove interfaces that are in significant 
use and should try to maintain backwards compatibility as much as possible.

That being said, since my original post I have found out that 
map.nextLabel() was never fully implemented in PHP MapScript, so this 
method is not used at all in PHP. And if PHP users never asked for it I 
doubt other MapScript users use it much. Did anyone on this list ever 
use map.nextLabel() or map.getLabel(int i) in SWIG MapScript?

Actually, what's the use of reading label cache members? What would a 
script do with them? I am trying to understand the use case and figure 
what use there is for a nextLabel() or getLabel() method if any.

Doing a search in Trac I found ticket 1794 which talks about the 
addition of getLabel(i) to solve the static var issue but doesn't 
describe the actual use case:
https://trac.osgeo.org/mapserver/ticket/1794

I was not aware of getLabel(int i) until I read Tamas' message. If we 
find real use cases for reading label cache members then I think 
getLabel(int i) is a better interface. I think nextLabel() should go 
away because it could never be practically used anyway since there was 
no way to reset the iteration and its state was global to the current 
process.


Finally, the reason why I bring up all that is that with the 
labelCacheObj becoming an array of cache slots for each priority level, 
labelCache.numlabels would disappear (and be replaced by a numlabels for 
each slot) so we are already going to break backwards compatibility for 
the very few users (if any) of getLabel(i) that rely on 
map->labelcache>numlabels.


I propose that, assuming there is a real use case for getLabel(i), we 
make the following changes required to support the new label priority 
stuff and solve the current issue:


1- In SWIG MapSCript:
   1a- Get rid of mapObj.nextLabel() as it's broken and nearly useless.

   1b- Deprecate mapObj.getLabel(int i), to be removed in a future release.

   1c- labelCacheObj.numlabels will be maintained artificially for 
backwards compatibility but will be deprecated, to be removed in a 
future release

2- In both SWIG and PHP MapScript:
   2a- Add labelCacheObj.getNumLabels()

   2b- Add labelCacheObj.getLabel(int i)

Does that sounds like an acceptable plan?

Daniel


Tamas Szekeres wrote:
> Frank,
> 
> I don't object keeping this function until some further releases as it
> stands now.
> However I don't favour introducing the practice of keeping much of the
> client related state (like the state of an iteration) inside the
> mapserver core.
> 
> Best regards,
> 
> Tamas
> 
> 
> 2007/7/1, Frank Warmerdam <warmerdam at pobox.com>:
>> Tamas Szekeres wrote:
>> >>
>> >> As the "Ghost of Backward Compatability" I'd rather not get rid of
>> >> getNextLabel() if it is in significant use without a very serious 
>> reason.
>> >> We might however make an effort to deprecate it in favor of
>> >> getLabel(int i).
>> >>
>> >
>> > Frank,
>> >
>> > The next major release is the best option to get rid of the unwanted
>> > parts of the interface.
>> > Moreover the current nextLabel is quite unconventional and does not
>> > allow the client to reset the iteration. I have no idea what is the
>> > desired use this function then. When we build up a loop until the
>> > returned value is null we might not get all of the results back. It's
>> > definitely unfeasible. Though we are thinking of that support of
>> > resetting the iteration, I guess it goes against the idea of the
>> > deprecation.
>>
>> Tamas,
>>
>> Well, I respectfully disagree.  If the interface works, and is in
>> reasonably widespread use, and we have no compelling reason for
>> removing it then I don't think we should remove it.
>>
>> Changing stuff like this is going to be a poke in the eye for folks
>> trying to maintain higher level applications, especially if they would
>> like them to work with 4.10 and 5.0.
>>
>> Deprecating normally involves advising that a particular interface is
>> no longer intended for use, but not actually removing it for some
>> time into the future (perhaps at MapServer 6.0).
>>
>> On the other hand, this might be a good time to remove some cruft
>> that we deprecated at 4.0.
>>
>> I'm not a genius programmer, and I'm not particularly skilled at
>> web applications.  But there is one thing that I know.  Interface
>> churn costs user-base, and a stable API shows love for the user-base.
>>
>> Best regards,
>> -- 
>> ---------------------------------------+-------------------------------------- 
>>
>> I set the clouds in motion - turn up   | Frank Warmerdam, 
>> warmerdam at pobox.com
>> light and sound - activate the windows | http://pobox.com/~warmerdam
>> and watch the world go round - Rush    | President OSGeo, 
>> http://osgeo.org
>>
>>


-- 
Daniel Morissette
http://www.mapgears.com/