[Laszlo-dev] Code Review: (LPP-2181) incubator component scrolledittext.lzx
jgrandy at openlaszlo.org
Tue Jun 20 11:54:30 EDT 2006
There's a naming principle at work here. In other projects it is
called "Key Value Coding" (KVC -- see http://tinyurl.com/jlsqj). The
idea is that programmers adopt a naming convention in which the names
of the setter and getter functions are derived from the name of the
affected attribute. So, if you have an attribute "foo", we expect
that there are functions named "setfoo" and "getfoo".
Our story is a little different since we have setAttribute/
getAttribute and are de-emphasizing direct calls to setter and getter
functions. But the principle is still important since folks do write
and read setter functions even though those functions aren't
typically called directly.
In order for KVC to have value, though, its important to avoid using
a name that looks like a setter or getter when the function does not
behave like a setter or getter.
In fact, evidence that "setvscrollwidth" is confusing is already in
front of us -- Phil assumed from the name that it was a setter, and
concluded that "v" and "_vs" were referring to the same value.
I'd recommend using "visibilityChanged" or "onvisibility" (if you
want to turn this into an event), or even
On Jun 20, 2006, at 8:42 AM, Michael Gregor wrote:
> This method is called by a delegate that tracks the visibility of
> the scrollbar. The text area's width is constrained to a factor
> of this width as well as the border property of the scrolledittext.
> The constraint was already named vscrollwidth, so we just wired up
> the new 'instantiated' delegate to it to replace the broken
> declarative constraint.
> On Tue, Jun 20, 2006 at 8:30 AM, Jim Grandy wrote:
>> Possibly reading this out of context, it's strange given our
>> setter naming conventions that the argument to a function named
>> "setvscrollwidth" should be "v" for "visible" rather than "v" for
>> "vscrollwidth". Might you consider renaming this function?
>> On Jun 19, 2006, at 9:53 PM, Michael Gregor wrote:
>>> v is for visible.
>>> the latter method of eval still stands though...
>>> this.setAttribute("vscrollwidth", (v ? this._vs.width : 0));
>>> 415 577 9184
>>> From: Philip Romanik To: mgregor13 at hotmail.com
>>> CC: laszlo-dev at openlaszlo.org
>>> Subject: Code Review: (LPP-2181) incubator component
>>> Date: Mon, 19 Jun 2006 22:40:52 -0400
>>> Code Review of scrolledittext.lzx
>>> 1. A piece of debugging code crept into the code (tstDbg in this
>>> 2. The event function is mixing variables. Both v and this._vs
>>> refer to the
>>> same variable, and they should not be used in the same method.
>>> For example:
>>> this.setAttribute( "vscrollwidth", this._vs.width);
>>> this.setAttribute( "vscrollwidth", 0);
>>> Consistent (and streamlined) version:
>>> this.setAttribute("vscrollwidth", (this._vs ?
>>> this._vs.width : 0));
>>> Laszlo-dev mailing list
>>> Laszlo-dev at openlaszlo.org
>> Laszlo-dev mailing list
>> Laszlo-dev at openlaszlo.org
More information about the Laszlo-dev