Puzzled by the Location and Reporting of #longMethodSize

Hello Forum,

For a method that has 10 lines, but 5 statements, Code Mentor is
reporting that the method is long.  It indicates that the setting can be
determined with the class method #longMethodSize in BasicLintRule.  But
I see the method only in BlockLintRule, and it reports the setting as
10.  But is that 10 lines or 10 statements?

Code Mentor explains that the size is based on statements, not lines.
Am I missing something?

Thanks.


Cheers,

Eric

Eric,

> For a method that has 10 lines, but 5 statements, Code Mentor is
> reporting that the method is long.

Are you sure it only has 5 statements ?  It sounds unlikely that Code Mentor
would loose count so easily ;-)


> It indicates that the setting can be
> determined with the class method #longMethodSize in BasicLintRule.  But
> I see the method only in BlockLintRule, and it reports the setting as
> 10.  But is that 10 lines or 10 statements?

That's the one.  And it's the count in statements.  Code Mentor thinks the
method BlockLintRule>>longMethods is itself too long (a little humour there
from the Refactory team ;-).  Experimenting with the value of #longMethodSize
shows that the RB thinks that #longMethods is exactly 10 statements long.
Since I count 10 statements (and 17 lines) in that method, it seems to be using
the same idea of what a "statement" is as I do.

Perhaps you have a different idea of what constitutes a statement.  How many do
/you/ see in BlockLintRule>>longMethods ?

    -- chris

Chris,

>Are you sure it only has 5 statements ?

Unfortunately, I'm sure of anything yet :).  Below is the code from
ESTMenu>>asMenu: aSymbol.  With the reading I've done over the past two
days, I'm now aware of ways in which I can optimize this code (more
expedient collections methods, avoiding far returns, etc.).  I just
haven't got round to it yet.  So please try not to laugh :).  And, yes,
you're looking at a piece of the menu abstraction layer I'm working on,
the one we discussed in an earlier thread.  I should have a visual layer
completed by Monday.  I've not included any exception-handling at this
point.

=======
self numberOfTopLevelMenus > 0 ifFalse: [^nil].
        menus do: [:each | each command == aSymbol ifTrue: [^each]].
        self numberOfMenus = self numberOfTopLevelMenus
                ifFalse:
                        [menus do:
                                        [:each |
                                        | found |
                                        found := each asMenu: aSymbol.
                                        found isNil ifFalse: [^found]]].
        ^nil
=======

I guess I counted a statement as either a fragment ending with a period,
or the last line of a method following a fragment ending with a period.
Is that not correct?

Cheers,

Eric

But
> > I see the method only in BlockLintRule, and it reports the setting
as
> > 10.  But is that 10 lines or 10 statements?
>
> That's the one.  And it's the count in statements.  Code Mentor thinks
the
> method BlockLintRule>>longMethods is itself too long (a little humour
> there
> from the Refactory team ;-).  Experimenting with the value of
> #longMethodSize
> shows that the RB thinks that #longMethods is exactly 10 statements
long.
> Since I count 10 statements (and 17 lines) in that method, it seems to
be
> using
> the same idea of what a "statement" is as I do.
>
> Perhaps you have a different idea of what constitutes a statement.
How
> many do
> /you/ see in BlockLintRule>>longMethods ?
>
>     -- chris
>

Eric,

I make that 11 statements in all.  For instance:

    menus do: [:each | each command == aSymbol ifTrue: [^each]].

has three statements.  It is one in its own right, but it contains a nested
statement:

    each command == aSymbol ifTrue: [^each]

which in turn contains another nested statement:

    ^each

Overall it has only 4 top-level statements, but the code critic counts the
statements inside blocks too.

(FWIW, if I were writing a long-method test I would use the number of syntactic
message sends as the metric, rather than the number of statements.  There are
15 in the above code.)

    -- chris

I wrote:

> Overall it has only 4 top-level statements

s/4/5/

    -- chris

Chris,

>Overall it has only 4 top-level statements, but the code critic counts
the >statements inside blocks too.

O.K., now I see.  The Code Critic sees Smalltalk statements much as they
are in other languages.  For example, x:=3 is a statement in other
languages, regardless of a period.

I would agree with you that the message-send metric might be more
telling.

Cheers,

Eric