The Artima Developer Community
Articles | News | Weblogs | Books | Forums
Artima Forums | Articles | Weblogs | Java Answers | News
Sponsored Link •

Weblogs Forum
Pardon My French, But This Code Is C.R.A.P. (2)

36 replies on 3 pages. Most recent reply: Sep 4, 2007 7:55 AM by Alberto Savoia

Welcome Guest
  Sign In

Go back to the topic listing  Back to Topic List Click to reply to this topic  Reply to this Topic Click to search messages in this forum  Search Forum Click for a threaded view of the topic  Threaded View   
Previous Topic   Next Topic
Flat View: This topic has 36 replies on 3 pages [ « | 1 2 3 ]
Peter Booth

Posts: 62
Nickname: alohashirt
Registered: Aug, 2004

Re: Pardon My French, Please Speak English Posted: Jul 22, 2007 6:22 PM
Reply to this message Reply
Advertisement
I would encourage anyone with suggestions about the details of the metric to first read http://www.bikeshed.com/

Sebastian Bergmann

Posts: 313
Nickname: sbergmann
Registered: Sep, 2004

Re: Pardon My French, But This Code Is C.R.A.P. (2) Posted: Aug 6, 2007 4:32 AM
Reply to this message Reply
I added the C.R.A.P. index to the metrics supported by PHPUnit (http://www.phpunit.de/).

Thank you for this metric!

Alberto Savoia

Posts: 95
Nickname: agitator
Registered: Aug, 2004

Re: Pardon My French, But This Code Is C.R.A.P. (2) Posted: Aug 6, 2007 7:32 AM
Reply to this message Reply
> I added the C.R.A.P. index to the metrics supported by
> PHPUnit (http://www.phpunit.de/).
>
> Thank you for this metric!

Hi Sebastian,

You are most welcome. I am impressed with your use of metrics in PHPUnit.

It's been a interesting few weeks since I posted about C.R.A.P. There are clearly a lot of strong opinions both in favor and against any software metrics - and lots of justified skepticism and unjustified fears (using metrics to "fire" people).

My position is that when it comes to metrics, we have thrown the baby out with the bathwater. Because there is no single perfect metric, because a metric can be misused and abused, we don't have any metric that can be considered a widely used, standardized, industry standard. Considering that software runs most of our world and it's a multi-$100B industry, I find this rather shocking.

Look forward to more posts and more work from me on the subject.

In the next few weeks will post an open-source Eclipse plug-in (crap4j) and we are continuing to experiment with the metric on Agitar code and lots of open-source code to refine it.

Thanks again for your work on metrics and support of C.R.A.P.

Alberto

Asgeir Storesund Nilsen

Posts: 1
Nickname: asgeirn
Registered: Aug, 2007

Re: Pardon My French, But This Code Is C.R.A.P. (2) Posted: Aug 26, 2007 7:38 AM
Reply to this message Reply
> I know that there will be exceptions (e.g. having 50
> methods in a StringUtil class might be OK - even the right
> thing to do). But, just out of curiosity and for a data
> point, what would you consider to be an acceptable
> threshold for # of methods in a class?

I would guess five to seven is a good starting point, both for method count and also statement count within a method.

This figure is based on the size of the human short term memory, which for most individuals fall within this range.

Other elements suited for inclusion in C.R.A.P. would be any of the other classic Code Smells like feature envy, God object, inappropriate intimacy, and more.

Andrew Binstock

Posts: 9
Nickname: binstock
Registered: Sep, 2006

Re: Pardon My French, But This Code Is C.R.A.P. (2) Posted: Sep 2, 2007 1:34 AM
Reply to this message Reply
The trouble with this metric is the reliance on cyclomatic complexity (CMC). CMC adds +1 for every case statement in a switch. So a switch with 40 cases has a CMC of 40. And in many cases, switches cannot be refactored to lower CMC. (What are you going to do--four switches of 10 cases each just to pass the metrics test?)

I am happy to use CRAP metric if you change CMC to: CMC - case statements.

(This factor should also be taken into account in the AgitarOne metrics dashboard, by the way, as it blindly flags clean code for violating CMC standards because of switch statements.)

Vincent O'Sullivan

Posts: 724
Nickname: vincent
Registered: Nov, 2002

Re: Pardon My French, But This Code Is C.R.A.P. (2) Posted: Sep 3, 2007 5:01 AM
Reply to this message Reply
> The trouble with this metric is the reliance on cyclomatic
> complexity (CMC). CMC adds +1 for every case statement in
> a switch. So a switch with 40 cases has a CMC of 40.

Of course, without seeing the code it's impossible to make a useful judgement on it but I'd say that a switch statement with forty branches is definitely something that should be flagged up strongly as a candidate for refactoring.

> And in many cases, switches cannot be refactored to lower CMC.

There is always more than one way to code any piece of logic.

> (What are you going to do--four switches of 10 cases each
> just to pass the metrics test?)

Perhaps that might be a resonable way to go. Are all forty braches really completely unique? Perhaps refactoring it to a state or strategy pattern might also be an option.

> I am happy to use CRAP metric if you change CMC to: CMC -
> case statements.
>
> (This factor should also be taken into account in the
> AgitarOne metrics dashboard, by the way, as it blindly
> flags clean code for violating CMC standards because of
> switch statements.)

Big switch statements are a symptom of procedural code. Before dismissing any measure that flags them up, as being flawed; you need to be really sure that they really are the best solution (and not just the quickest and simplest to code).

Alberto Savoia

Posts: 95
Nickname: agitator
Registered: Aug, 2004

Re: Pardon My French, But This Code Is C.R.A.P. (2) Posted: Sep 4, 2007 7:55 AM
Reply to this message Reply
Hi Andrew,

Thank you for the suggestion. I can see you know your Cyclomatic Complexity well :-).

We looked at a lot of code, and case statements did stick out as something that might deserve a special treatment. The reason we did not rush to give case statements such treatment in version 0.1 of the metric is that there are examples - lots of them - where it could be replaced by something simpler, safer, and more elegant.

The canonical example of this is uses of the case statements to map, say, state abbreviations (e.g. CA) into state names (e.g. California) or state tax calculations (e.g. 8.25 in CA). We've seen several examples of people doing this with 50+ case statements. Is this the best way?

Some people argued that a HashMap would be a better way to implement this and that, generally speaking, if you need tens of case statements to accomplish something, that might be an indication that there might be a better design lurking somewhere.

What do you folks think. Anyone willing to make a case for the case statement NOT receiving preferential treatment?

Actual examples would be most compelling.

Alberto

Flat View: This topic has 36 replies on 3 pages [ « | 1  2  3 ]
Topic: Pardon My French, But This Code Is C.R.A.P. (2) Previous Topic   Next Topic Topic: Python Bites (off-topic)
Sponsored Links


Google
  Web Artima.com   

Copyright © 1996-2019 Artima, Inc. All Rights Reserved. - Privacy Policy - Terms of Use