WATagBrush>>#tag should be class side?

I'm playing with a small utility that walks the canvas and the
hierarchy of WATagBrush, and one thing I find is that #tag method is
implemented on the instance side, whilst the class side requires a new
instance to obtain the HTML tag name.

In most cases this isn't an issue since brushes are simple, but in the
case of WABasicFormTag it tries to get the encoding from the request
context and you fail to obtain the tag unless within a context.

Since the returned value doesn't change based on conditions of the
brush [1], shouldn't #tag be implemented class side and instances
delegate to the class by default and implement only if necessary?

As far as I could see the change would be idempotent, even third party
subclasses of WATagBrush would continue to work if they don't
implement #tag class-side.

What do you think?

Esteban A. Maringolo

[1] The only case I found is the heading tag, that uses h1, h2, h3,
etc.;and it would continue to work.
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

In general, I agree. I’m not sure what the performance costs might be, however. It’s only one message send but they add up to a lot (context creation isn’t free, unfortunately). Could you try and run a benchmark?

Max

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

2017-05-29 12:51 GMT-03:00 Max Leske <[hidden email]>:

>> On 29 May 2017, at 17:24, Esteban A. Maringolo <[hidden email]> wrote:

>> Since the returned value doesn't change based on conditions of the
>> brush [1], shouldn't #tag be implemented class side and instances
>> delegate to the class by default and implement only if necessary?
>
> In general, I agree. I’m not sure what the performance costs might be, however. It’s only one message send but they add up to a lot (context creation isn’t free, unfortunately). Could you try and run a benchmark?

There seems to be a significant difference in the execution of these,
although I could bet that given the insanely fast execution of that,
it would be completely imperceivable.

Object subclass: #WATestTag.

WATestTag>>#tagClass
  ^self class tag

WATestTag>>#tagInstance
  ^'tag'

WATestTag class>>#tag
  ^'tag'


brush := WATestTag new.
{
  [ brush tagInstance ].
  [ brush tagClass ].
  [ WATestTag tag ]
 } collect: [ :each | each bench ].

 "#('88,233,008 per second' '48,986,375 per second' '83,564,833 per second')"


To get a "workable" profile sample I had to repeat the execution 10^8 times.

#tagClass
**Memory**
  old +0 bytes
  young -723,840 bytes
  used -723,840 bytes
  free +723,840 bytes

**GCs**
  full 0 totalling 0ms (0.0% uptime)
  incr 1 totalling 0ms (0.0% uptime), avg 0.0ms
  tenures 0
  root table 0 overflows

#tagInstance
**Memory**
  old +0 bytes
  young +830,776 bytes
  used +830,776 bytes
  free -830,776 bytes

**GCs**
  full 0 totalling 0ms (0.0% uptime)
  incr 0 totalling 0ms (0.0% uptime)
  tenures 0
  root table 0 overflows



What you guys think?


Esteban A. Maringolo
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

On Mon, May 29, 2017 at 5:24 PM, Esteban A. Maringolo
<[hidden email]> wrote:
> I'm playing with a small utility that walks the canvas and the
> hierarchy of WATagBrush, and one thing I find is that #tag method is
> implemented on the instance side, whilst the class side requires a new
> instance to obtain the HTML tag name.
>
> In most cases this isn't an issue since brushes are simple, but in the
> case of WABasicFormTag it tries to get the encoding from the request
> context and you fail to obtain the tag unless within a context.

you can try #basicNew which doesn't send #initialize or check if the
source is a simple return of a string literal.

There are more that won't work like WAGenericTag.

As said before there are several cases that don't work and tooling
needs work arounds for those anyway. So there seems to be no again.

To me the biggest argument against is that it's behaviour of the
instance and not the class and should therefore be on the instance
side.

Cheers
Philippe
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

+1

>
> Cheers
> Philippe
> _______________________________________________
> seaside-dev mailing list
> [hidden email]
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

Hi Phil, all,

2017-05-30 3:19 GMT-03:00 Philippe Marschall <[hidden email]>:

>> [1] The only case I found is the heading tag, that uses h1, h2, h3,
>> etc.;and it would continue to work.
>
> There are more that won't work like WAGenericTag.

WAGenericTag class>>tag currently returns nil, which currently doesn't
work as well, so in this particular case the behavior would be
preserved.

> As said before there are several cases that don't work and tooling
> needs work arounds for those anyway. So there seems to be no again.
> To me the biggest argument against is that it's behaviour of the
> instance and not the class and should therefore be on the instance
> side.

I'd argue the opposite, since most returned values (tag strings) are
the same for all the instances of the same class; with the already
mentioned exception of heading and generic tag. I tend to define such
behaviors class side, but it is a matter of coding style [*].

In the case of WAHeadingTag class>>#tag it returns 'h1', which is a
reasonable default, and I already mentioned the case of WAGenericTag.

I'm fine with whatever you choose, I'm not a core developer of
Seaside, but my question was raised in the context of tooling, when I
was trying to map tags to brush classes and canvas selectors (e.g. 'a'
-> WAAnchorTag or #anchor).

As you said I'll use the #basicNew workaround to avoid any
initialization, it seems to work so far.

Thank you for you input!

--
Esteban A. Maringolo

[*] I particularly find "wrong" (taste/stylewise) the need to
instantiate an object, which in many cases is not properly
initialized, to obtain a value that is not computed using anything
related with the instance, I feel the same with Magritte 3 that
requires you to instantiate an object just to get a MADescription. I
like to be able to query the class for "general" behavior, and then
implement everything instance side overriding when appropriate and
delegating to the class side by default.
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev