On 11-1-2014 19:13, Orchid Win7 v1 wrote:
> Over Christmas, I read a book called "Clean Code". My employer lent it
> to me. No, I don't remember who the authors were.
>
> While I don't agree with absolutely everything the book said, it does
> seem to speak a great deal of sense. The book says obvious things like
> DRP and SRP. It says things like "name stuff sensibly" and "don't write
> 500-line methods". But it also says more insightful things.
>
> For example, don't mix multiple levels of abstraction in a single block
> of code. Now I don't know about you, but I know for a fact that I have
> *totally* written code where I'm like "OK, so I process each item, and
> oh wait, I need to dump out some HTML here. Oh well, I'll just quickly
> hard-code it in here. It's only small anyway..." It turns out, even for
> small things, actually separating stuff out into a few tiny methods with
> descriptive names *really does* make it that much easier to read the
> code again later. Even if it seems pointless to implement a whole
> separate method just for a few lines of code.


My rule of thumb is that whenever a piece of code does a single thing 
that you can write a specification for that is independent of the 
context, it should be a subroutine/function/method. With a name that 
describes that specification.
Which would mean that because processing can be specified, and 
generating output can be specified also, and both specifications are 
independent, I would end up with two functions and a wrapper.


I am working with another group that has apparently a different opinion 
on writing code. Their code has numerous instances of this code snippet

     S=getSmode(T,dep,rep, GEOM.SPECS ,4);
     PHI=lowpassma(GEOM.AMA*S,5);	

this is a short version, there are others that do it in one line, take 
subsets and do other stuff as well:

    PSIA =lowpassma(SCAN.AMA * getSmode(ones(length(GEOM.VER),1) * ( 1 : 
SCAN.qrsduration ),dep,SCAN.rep, GEOM.SPECS,SCAN.scanmode,GEOM),SCAN.lpass);

(never mind that most variables have different names in this place for 
no obvious reason, it still does the same thing. There are many more 
unacceptable things in this snippet of code, but let's stick to the 
naming issue here.)

Question: what do you think this does?
Answer below
Would you create a separate function for this, even if it is just one or 
two lines and how would you name that function?

>
> Possibly the best insight the book has to offer is "if you need to read
> the comments to work out what the code does... the code sucks". The
> goal, basically, is to write your code in such a way that it's so
> utterly *obvious* what it does that there's really no need to write
> comments about it. The book goes into quite some detail on circumstances
> where it *is* valid to include comments - e.g., to document obscure
> quirks of external code you're interfacing to or something. But the main
> argument is that you should simplify your code to the point where
> comments become superfluous.

I fully agree, but see my other post

> Reading through the book, and looking at the examples, and so on, the
> thing that impressed me is how widely applicable the advice is. The book
> is written in Java, but the advice is (almost) all equally applicable to
> any OO language. But actually, most of this stuff would apply just as
> well to Pascal or Bash scripts... or Haskell.

Looks like this book is on programming in general. In my opinion there 
is a huge difference between being able to debug a program until it sort 
of works and being a competent programmer. This book is for the latter 
and will be totally lost on the former.

> There are a few things I don't agree with. The book asserts that a
> method should never have a bool argument. Because "if it has a bool
> argument, it does two different things, and you should change it into
> two separate methods instead". Yeah, and are you going to split every
> single client of this method into two methods as well??

I should read the book first, but I agree with your statement. If

  foo(bool,args)

is replace by

  if (foo)
    footrue(args)
  else
    foofalse(args)
  end

always, then I am of the opinion that these 5 lines should be taken 
together as one method, because there is a specification for this entire 
block.
Then either these five lines will be the body of that method, or in case 
they have overlapping computation you should do that first.
BTW in case of overlapping computations in footrue and foofalse that can 
not be logically separated from the part that differs, you should not 
use the footrue/foofalse methods at all. That would imply duplicating 
code that from then on might start to evolve separately, creating 
problems later. (alternative: use literate programming ;) )


> Similarly, you
> should never pass an enum to a method, because then it does *multiple*
> things. Again, this seems silly if all the method does is pass that enum
> on to somebody else. And even if it's dealt with locally, I don't see
> how this is "fundamentally bad" (although often you can replace a big
> switch-block with some nice OO polymorphism instead of using enums).
>
> People often complain about Haskell's heavily abbreviated variable
> names. The book says that variable names should be descriptive, but it
> also states that the level of description should be proportional to the
> size of the variable's scope. So if you have a global variable (i.e.,
> you are evil), it should have a *really* damned descriptive name. But if
> you have a loop counter that's only in scope for 3 lines of code, it's
> find to use something shorter.

My rule is: use descriptive names when possible and random names when 
that is not or when it does not matter. The latter prevents you from 
trying to understand why somebody named a variable thus, when it 
actually does not matter.

> Haskell of course *abounds* with 1-line function definitions. If a
> variable is only in scope for one single line of code, how much of a
> name do you really need? Similarly, when you have a function like "flip"
> that takes a 2-argument function and swaps its two arguments, you could
> name those arguments as "x" and "y", or you could call them
> "first_argument" and "second_argument". But how is that better? The
> shorter names are quicker to read, and it's arguably easier to see
> what's going on. And that's a common theme in Haskell: often you write
> code that's so utterly abstract that it would be difficult to come up
> with meaningfully descriptive names in the first place.
>
> Heavily abbreviating function names, however, is rather less easy to
> justify. Was it *really* necessary to shorten "element" to "elem"?
> Similarly, the book demands that names *tell you* something about the
> items they refer to. The "max" and "maximum" functions are *clearly*
> different functions - but the names yield no clue has to *how* they are
> different. There _is_ some logic there, once you look it up, but it's
> hardly self-describing. ("max" finds the maximum of just two arguments,
> "maximum" finds the maximum element of an entire list of items. So one
> has a short name, and the other a long name. Logical, but not really
> self-describing.)
>
> The same could maybe levelled at the standard class names. We have "Eq",
> "Ord" and "Num" rather than "Equal", "Ordered" and "Number". And
> "number" is a crappy name anyway; Num defines +, - and *, and also abs,
> signum and conversion from Integer. But "/" is defined in Fractional.
> (Not "Frac", thankfully.) Then again, designing a decent number
> hierarchy is *highly* non-trivial, so...
>
> The "id" function could *easily* have been named "identity" instead.
> That would have been way, way more descriptive.

Many of the examples you give are rather common and very ancient, so I 
would be surprised at the longer version.

-----------

Answer: it computes an estimate of the surface ECG from a set of 
activation and recovery times of parts of the ventricle.


-- 
Everytime the IT department forbids something that a researcher deems
necessary for her work there will be another hole in the firewall.

Post a reply to this message