On 2006-05-12, at 00:15 PDT, Adam Wolff wrote:

> On May 12, P T Withington wrote:
>>>> There is one issue that I'd like some input before  I make any
>>>> changes.  Here's the snippet of code:
>>>>     if (args.height == null &&
>>>>         typeof(args.$refs.height) !=  "function") {
>>>>          ...
>>>>     }  else {
>>>>          textclip._height = args.height;
>>>>     }
>>>>
> [snip]
>> I'm not sure I follow the intent of the code here.  Is it testing for
>> args.height being null, or for it not existing?
>
> Comments are overrated. I think the intent here is that text fields  
> size
> to their measured height if none is given; otherwise they clip to  
> their
> height. This is different from the way views normally act, but it  
> doesn't
> seem like you should have to say
>     <text height="120" clip="true"

Yes comments are overrated.  Especially when they don't explain the  
code.  There is already a nice comment on this code, but I have no  
idea what the `typeof(args.$refs.height) !=  "function")` is trying  
to say from that comment.

I think there are at least four things going on here that make it  
extra confusing.

1) The text object might be created without a height argument

2) There is an idiom where setting the height explicitly to null  
means to 'unset' the height, to have the object take on the height of  
its contents

3) There is an even more obscure idiom where a subclass of a class  
that has constrained its height may override the constraint with a  
literal value

4) The original author may not have considered that the undefined  
value and the null value are ==

---

To solve 1, we'd like to ask `height in args`, but we can't (yet) so  
we have to know whether `args.hasOwnProperty` would do the job (i.e.,  
is args a straight object, or does it use __proto__ inheritance?).   
Or, we can use our current idiom of `args['height']`.  BUT, to solve  
2, we need to distinguish between undefined and null, since null is a  
special marker for the height (and width) properties, and 4, `==`  
does not distinguish, only `===`.  3) might be made less obscure with  
a lot of code rework, but surely just a teeny comment "see  
LzNodeInitialize" perhaps, might give subsequent maintainers a clue?

> You could probably fix this elegantly by setting clip="true" on the
> LzText6.prototype, but that might be some doing. It would be  
> helpful to
> just have a version that runs without warnings. You could write:
>     //TODO: This is a stupid way of saying that a text with set height
>     //clips
>     if( args[ "height" ] ...

What if I create a text object with `height="null"`?
_______________________________________________
Laszlo-dev mailing list
[email protected]
http://www.openlaszlo.org/mailman/listinfo/laszlo-dev

Reply via email to