Creating classObj doesn't automatically set 'numstyles' to 1

[jwwilhit@…]

When I create a classObj the default value of the attribute 'numstyles' is 0.  
Since I can assign values to the 'styles' attribute of the classObj all day 
long it would seem to me that a styleObj is being created automatically when a 
classObj gets created.  This would lead me to believe the default value for 
the 'numstyles' attribute of the classObj should be 1 instead of 0.

If you don't explicitly set the 'numstyles' attribute to 1 then any of the 
values you assign to the 'styles' attribute won't be rendered.  It will let you 
set them but they won't get drawn.

I'm including the email track and code snippet below:

-------------- Code -------------------

my $tclass = new mapscript::classObj( $warnLayer );
print "numstyles = $tclass->{numstyles}\n";
$tclass->{numstyles} = 1; ######## This has to be set in order for the stuff 
below to work

$tclass->{name} = "Dynamic Class";
$tclass->{status} = $mapscript::MS_ON;
$tclass->{styles}->{symbol} = 1;
$tclass->{styles}->{outlinecolor}->{red} = 250;
$tclass->{styles}->{outlinecolor}->{green} = 0;
$tclass->{styles}->{outlinecolor}->{blue} = 0;
$tclass->{styles}->{color}->{red} = 0;
$tclass->{styles}->{color}->{green} = 250;
$tclass->{styles}->{color}->{blue} = 0;


----------- Email Track ------------------
> That was it!  Thank you very much.  Although...It would seem to me 
> that that attribute should be set to 1 by default when a class gets created.
> Apparently a styleObj gets created when you create the class because I 
> was able to set the styleObj attributes all day long but the numstyles 
> attribute remains 0.
>
> Talk about a stealth requirement.
>
> Thanks again.
>
> -Jason
>
> -----Original Message-----
> From: mapserver-users-admin@lists.gis.umn.edu
> [mailto:mapserver-users-admin@lists.gis.umn.edu] On Behalf Of John 
> Beisley - RSG
> Sent: Wednesday, March 31, 2004 1:42 AM
> To: mapserver-users@lists.gis.umn.edu
> Subject: Re: [Mapserver-users] Adding Dynamic layers in Mapscript 
> 4.0.1 Perl
>
> WILHITE JASON W wrote:
>
>> <>The static members of the map file are showing up just fine but not 
>> the dynamic layer. I have a feeling it might be a problem with the 
>> color assignments but for the life of me I can't figure it out.
>>
>> Does anyone have any ideas?
>>
>> Thank you,
>>
>> -Jason
>>
> I think I ran into a similar problem myself. Although I don't recall 
> the
>
> solution, I do remember finding that calling save() on the map object 
> to
>
> save the generated map file to disk was a very useful debugging 
> solution.
>
> However my own code for setting the style on a line layer that I have 
> does something similar to the following: (where $layer is a layerObj)
>
> my $layerClass = new mapscript::classObj($layer);
>
> # ... Set the values inside $layerClass ...
>
> $layerClass->{numstyles} = 1;
>
>
> - John
>

[sgillies@…]

Fixed.  The classObj constructor now sets numstyles=1.

Thanks for the exhaustive bug report Jason.

Committed to CVS and will be in the next nightly and the 
next release.

[dmorissette]

Is this really the right thing to do?  A classObj automatically has MS_MAXSTYLES
styles allocated in it, so in theory you can write to the second or third style
the same way.

I think that to be consistent with other stuff such as classes inside a layers
which are also preallocated in C, but you have to explicitly allocate from
MapScript, it should be up to the mapscript code to call a function to allocate
the first style.

I'll CC Steve to get his opinion.

[sgillies@…]

Daniel, you're right.  Am reopening the bug.  What do you think about this
example for proper usage in 4.2:

    new_class = mapscript.classObj(layer)
    new_style = mapscript.styleObj()
   
    # set style attributes
    new_style.color.red = 204
    # ... 
   
    # insert style by default at index numstyles (0 in this case)
    # this method also increments numstyles
    new_class.insertStyle(new_style)

And then we should try to deprecate the API introduced in 4.0 that
the bug reporter is using.  We could hide a classObj's styles 
attribute from SWIG in the same way that we don't allow drirect
access to a layerObj's classes?

While we are at it, I should make numstyles immutable so that the
only way to increment or decrement is through insert/removeStyle
methods.

[dmorissette]

I agree numstyles should be immutable.

About the method that you propose, could we not make it similar to the way
layers are added to maps, and classes are added to layers? e.g.

    new_class = mapscript.classObj(layer)
    new_style = mapscript.styleObj(new_class)
    # set style attributes
    new_style.color.red = 204
    # ... 
   
This seems simpler unless there is a good reason to have standalone style
objects? That's also the way PHP MapScript is currently implemented.

Anyway, if style objects really have to be standalone, then the method
insertStyle(new_style) should probably be renamed to addStyle() since it adds to
the end of the styles array and not to the begining.

[sgillies@…]

Actually insertStyle takes an optional index argument which lets us insert
in the range [0, MS_MAXSTYLES).  An 'addStyle' method that does none other
than insertStyle() could be convenient.  Aside: I don't like the way we've been
using 'add' for an operation that is more accurately an 'append', so I'd
actually prefer 'appendStyle'.

See bug 548.

Why no classObj argument for the styleObj constructor?  I have a number of
use cases for a stand-alone styleObj (and even *more* for stand-alone layerObj).
So I think the parent mapping object argument should be optional if it is
there at all.  Maybe I'll go back and implement that now.

[sgillies@…]

OK, here is the solution.  Am going to close the bug.

*STOP* accessing class styles this way

    the_class.styles.color.red = 100
    ...

and start accessing class styles in the new manner which is more inline with
the use cases for layers and classes

    the_style = the_class.getStyle(style_index)
    the_style.color.red = 100
    ...

For new classObj instances, do this

    new_class = mapscript.classObj(the_layer)
    new_style = mapscript.styleObj(new_class)
    new_style.color.red = 100
    ...

This is the way to do it and in a future version we are going to make 
numstyles immutable so that programmers cannot modify it directly.  Will
also be removing a classObj's styles attribute from the API.  I'm
creating issue 611 for discussing this.

Meanwhile we will continue to let programmers access 'styles' directly,
but this use case is discouraged.  Programmers are on their own and will
have to increment numstyles "by hand".

[dmorissette]

Sold...and that's in line with the way php mapscript works as well.