Fortran-FOSS-Programmers / Best_Practices

the opinionated *best practices* of Fortran FOSS Programmers group
Creative Commons Attribution Share Alike 4.0 International
64 stars 10 forks source link

do we avoid dead code? #13

Open szaghi opened 8 years ago

szaghi commented 8 years ago

From a note on another thread...

avoid dead code, do not write code that has no effect

Is this an agreed guideline?

The discussion is open :smile:

Summary from other thread

@zbeekman

  • about select constructs if it's actually impossible to enter that branch of the select statement (even with potentially incorrect user input, corrupted data files, or out of memory errors) then it is "dead" code, and exists for no other reason than to give you a warm fuzzy feeling that you have been a diligent defensive programmer. Additionally this also means there is no way to test its correctness.

  • strive to get 100% test coverage during unit tests

  • don't build it until you need it

    @rouson

  • Dead code also violates the agile development method of test-driven development (TDD) in which one writes only enough code to pass the test. The test is the specification for what must be written. If one wants more code to be written, then one must say so in a test.

    @tclune

  • about select constructs I’d recommend “always” having a default block, but putting it at a low priority.

    @LadaF

  • My practice is to add the name after the end subroutine if the subroutine gets longer than one page in my editor. It's a subjective criterion and sometimes I forget and add it later when I find end subroutine for which I can't immediately see the beginning with the name.

    @nncarlson

  • avoid dead code

  • about select constructs If the intent is that one of the case stanzas is always executed but it is not completely clear from the immediate context that this will be the case, then I do add the default clause with an assertion:

case default
      ASSERT(.false.)
end select

Here ASSERT is part of a simple macro-based DBC system that I use. ASSERT asserts that its argument is true, so the strange idiom ASSERT(.false.) will trigger an error. In this case the default clause serves to document to the reader the intent that one of the case clauses is supposed to be executed.

nncarlson commented 8 years ago

I agree with this as a general guideline. However I think it is really important to remember that all of these guidelines/best practices we are wrangling over are just out workings of fundamental principles that we would most likely all be in agreement with (think torah vs talmud for those of you theologically inclined). There are always going to be those instances where guidelines should be violated to do something that adheres better on balance with the principles. In fact I think it would be a good idea to provide a rationale for each guideline that points to one or more of these basic principles.

szaghi commented 8 years ago

@nncarlson sorry, but my English understanding is very limited, my lexicon annoverates just technical English (a very limited subset of).

I think it is really important to remember that all of these guidelines/best practices we are wrangling over are just out workings of fundamental principles

What do you mean with out workings of fundamental principals?

think torah vs talmud for those of you theologically inclined

Does this means something like torah => guidelines as talmud => Fortran standard syntax?

There are always going to be those instances where guidelines should be violated to do something that adheres better on balance with the principles.

Absolutely agree (I think :smile: )

I think it would be a good idea to provide a rationale for each guideline that points to one or more of these basic principles.

I am not sure which principles you are referring to, can you give me an example?

nncarlson commented 8 years ago

@szaghi, my apologies! I need to be more mindful that there are non-native english speakers here.

A silly example for the purpose of illustration only. A fundamental principle might be "Code should be readable." (something no one would disagree with I hope). Guidelines that come from that (the "out working") might be guidelines about line length. The truly important thing is not "80 char max" but "readability", and situations might arise where "readability" is better served by violating the "80 char" guideline. Oh, and torah == principles, talmud == guidelines.

szaghi commented 8 years ago

@nncarlson

I need to be more mindful that there are non-native english speakers here.

Do not worry, only with me try to speak slow, I am the only donkey here :smile:

Wonderful example, now it very clear. I think that we could found a more clear form at to highlight which sentences are general principles and which are guidelines.

Thank you also for torah/talmud clarification (it is really interesting for me)

Tobychev commented 8 years ago

I don't think that "don't write dead code" is a good principle, its intent is better captured by something like "write elegant code, not superfluous code". Further I don't think that any of the rules suggested above follow naturally from the principle that they are supposed to support, but spring from some other largely unrelated principle (eg ttd is more about correctness by testing that it is about minimizing code lines, what it does say in that vein stems from the desire to have no untested code).

Mainly I think that "dead code" is a bad concept for high level principles because it not a general idea, as far as I know it is mainly a rather closed technical term from the world of compiler construction. On the language level, I don't think there is any concept of code with no effects beyond the comment statements; searching trough a draft of the 2003 standard most code seems to have effects except for a small number that has no effect in certain circumstances (the flush instruction for an non-existent file for instance). Calling a rule that only has something to say about "end if" statements (the execution of which the standard draft says has no effect) and superfluous "import none" statements a principle seems quite a stretch!

If we allow ourselves to include the execution of a piece of fortran code on an imagined fortran machine (thereby sidestepping the issues of how to regard the actions of the various compiler binaries) we can have a well defined notion of dead code: it is code that is never reached trough the repeated execution of or program over its complete range of possible inputs. Clearly writing this code was a waste. At the same time any reasonably complex piece of fortran code will show unexpected and unintended behaviour when tested over its complete range of possible inputs; it will have bugs. On of the fundamental sources of bugs is the programmer misjudging the state of variables, something that has critical effects at branch time. And I expect the main source of dead code is branches that are never taken ( the other source being that leap-frog integrator you wrote and tested but then never actually included in production; and other such bits and bobs). So it seems fairly likely that much dead code will only be recognized as such after careful examining of the program, and in the typical case this insight will lead to swearing and modification of the program until the test works as intended because nobody actually wants to write non-trivial pieces of dead code.

With this I mean to say that the only non-trivial pieces of dead code (such as an default statement "for safety") are unintentional, so that even if we look at the execution context the no-dead-code principle only has opinions on trivial examples (because the programmer will most likely not correctly identify the dead code except in trivial examples).

szaghi commented 8 years ago

@Tobychev I am sorry for my delay, I am traveling for work. Asap, I will try to summarize your arguments. See you soon.