Coding guidelines question
in [Fortran]
|
From: James Giles on 13 Mar 2006 21:38 Walt Brainerd wrote: > James Giles wrote: .... >> Most things about module use issues are not much different >> than IMPLICIT NONE. All the arguments against the ONLY >> clause sound very much like the arguments against IMPLICIT >> NONE earlier on. >> > I agree in general, but when I tried to force myself > to do it, one of the first examples was a substantial > module with new types and *many* operator and assignment > extensions. I found listing them much more painful than > just listing a bunch of variable names. You are using *all* those operators and assignments in each local context? You must have some very busy code. -- J. Giles "I conclude that there are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies." -- C. A. R. Hoare
From: James Giles on 13 Mar 2006 21:41 Paul Van Delst wrote: .... > Like I mentioned before, for the situation where there are many > developers (e.g. 5-10+) working on various aspects of the same code I > think there's a threshold beyond which it becomes onerous to list all > the required imported stuff from a USEd module via an ONLY. Then > again, maybe that's a good thing? Would it encourage people to write > code that is broken down into smaller subsets of functionality rather > than tossing a whole bunch of stuff into one monster(ish) module? And > even then, will that spread the ONLY "declarations" /down/ the > heirarchy or /across/ more, smaller modules Well, you are still assuming that the USErs of modules are themselves monsters if they need to reference a large fraction of the monster module's entities. In order for ONLY to be onerous, BOTH the modules and the USEr have to be monster sized. -- J. Giles "I conclude that there are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies." -- C. A. R. Hoare
From: Bil Kleb on 13 Mar 2006 23:06 Paul Van Delst wrote: > > Like I mentioned before, for the situation where there are many > developers (e.g. 5-10+) working on various aspects of the same code I > think there's a threshold beyond which it becomes onerous to list all > the required imported stuff from a USEd module via an ONLY. We haven't reached that point at 600k LOC with 3-4 heavy committers out of 12 or so total contributers. And recently we found the USE-ONLY construct a nice hook to do some software analysis along the lines of Lakos: We wanted to find out where to split some modules to break some physical dependencies. Having USE-ONLYs allowed us to do some histograms of module procedure and type usage that revealed where to make the splits. I have attached the code we used after my signature. Regards, -- Bil Kleb http://fun3d.larc.nasa.gov #!/usr/bin/env ruby # # For a given directory, this routine will find the use-only # patterns in resident *90 files, or those files given as # command-line arguments. # # Standard error output will contain the files and matches as each # file is parsed, while standard output will contain the frequency # counts for the various clusters of used routines for each module. # # Sample usages: directory, single file, multiple directories # # cd /path/to/FUN3D_90 # ../Ruby/useonly_patterns.rb > FUN3D_use-only_patterns.txt # # cd /path/to/Party # ../Ruby/useonly_patterns.rb gather_module.f90 > GATHER_use-only_patterns.txt # # cd /path/to/HEFSS.rps # ./Ruby/useonly_patterns.rb \ # `find FUN3D_90 Party PartyMPI Design GridMove GetGrad Adjoint -iname \*.f90`\ # > ALL_use-only_patterns.txt # # Note: This routine will include the results from symbolically-linked # files and will ignore 'use' statements without the 'only' qualifier. files = ARGV.empty? ? Dir["*90"].delete_if{ |f| !File.exists?(f) } : ARGV useonly = Hash.new $stderr.puts $stderr.puts "Files and their USE Statements" $stderr.puts "==============================" files.each do |file| $stderr.puts $stderr.puts "FILE #{file}" lines = IO.readlines(file) while (line = lines.shift) do $stderr.puts " #{$3.upcase} #$4" \ if line.match(/^\s*(?!(end|!|'|"))(|\S+\s+)(program|module|subroutine|function)\s+(?!procedure)(\w+)\W/i) if line.match(/^\s*use\s*(\w*),\s*only\s*:\s*(.*$)/) mod_name = $1 mod_routines = $2.gsub(/\s+/,'').gsub(/(&|!).*$/,'') while line.match(/,\s*&.*$/) line = lines.shift mod_routines += line.gsub(/\s+/,'').gsub(/(&|!).*$/,'') end mod_routines = mod_routines.split(',').sort $stderr.puts " #{mod_name}: #{mod_routines.join(', ')}" if useonly.has_key? mod_name then useonly[mod_name] << [mod_routines] else useonly[mod_name] = [mod_routines] end end end end $stderr.puts puts puts "USE Cluster Frequencies" puts "==========================" useonly.each_key do |mod| occurrences = Hash.new(0) useonly[mod].each do |routines| occurrences[routines.join(', ')] += 1 end puts puts mod.upcase occurrences.each do |cluster,frequency| puts "#{frequency.to_s.rjust(4)}: #{cluster}" end end puts puts puts "Ranked USE Cluster Frequencies" puts "===========================" ranked_useonly = Hash.new useonly.each_key do |mod| occurrences = Hash.new(0) useonly[mod].each do |routines| occurrences[routines.join(', ')] += 1 end ranked_useonly[mod] = occurrences.sort{|x,y| y.last <=> x.last}.first end longest_mod = ranked_useonly.keys.map{|e| e.length}.max ranked_useonly.sort{|x,y| y.last.last <=> x.last.last}.each do |tuple| mod, routines, frequency = tuple.flatten puts "#{frequency.to_s.rjust(4)} #{mod.upcase.rjust(longest_mod)}: #{routines}" end puts
From: Pierre Asselin on 14 Mar 2006 11:32 Paul Van Delst <Paul.vanDelst(a)noaa.gov> wrote: > I don't really think [USE, ONLY is] like an implicit none since the > entities in question aren't "in" the module in question. With > implicit none you declare variables etc in the file in which they > are, well, declared. With USE, ONLY: you have "declare" the imported > entities in every file that uses that module. So think of it as redeclaring the parts that you are about to (ahem) use in client code. The USE, ONLY is about the client code, not the module. > Like I mentioned before, for the situation where there are many > developers (e.g. 5-10+) working on various aspects of the same code > I think there's a threshold beyond which it becomes onerous to list > all the required imported stuff from a USEd module via an ONLY. You'll only find out by trying it, won't you ? One thing, though: the payoff doesn't come when you write the code. The payoff comes when you *read* the code (and also when you modify it). If anything, having multiple developers should shift the threshold *in favor of* USE, ONLY per routine. You impose added discipline at the time of writing new code --which has a real cost-- in order to simplify future development. > Then again, maybe that's a good thing? Would it encourage people > to write code that is broken down into smaller subsets of functionality > rather than tossing a whole bunch of stuff into one monster(ish) > module? Havent' you got this one backward ? USE, ONLY is even more important with monster modules since it allows you to specify the subset of the monster on which you actually depend. The small modules are the ones that you could USE wholesale (if you don't expect them to grow). Then again, if they are so small, why not add the ONLY clause as a documentation aid to the client code ? The only exception left is then the genuine monster module where every use will refer to a sizeable portion of the interface. Where that threshold lies I have no idea. > And even then, will that spread the ONLY "declarations" > /down/ the heirarchy or /across/ more, smaller modules So? USE statements in module procedures are a part of the implementation, not the interface. They don't impact the design. Since this was about coding standards, here are a few thoughts. 1) Nobody likes coding standards, but if they get over it they'll get over it. I speak from experience. 2) Just make sure the standards are not harmful. I think *mandating* USE at module level would be harmful. 3) You need a mechanism to grant waivers, because you can't be sure your standards won't be harmful rather than just annoying. 4) You need a mechanism to amend the coding standards, for the same reason. -- pa at panix dot com
From: Paul Van Delst on 14 Mar 2006 11:55
Pierre Asselin wrote: > Paul Van Delst <Paul.vanDelst(a)noaa.gov> wrote: > > >>I don't really think [USE, ONLY is] like an implicit none since the >>entities in question aren't "in" the module in question. With >>implicit none you declare variables etc in the file in which they >>are, well, declared. With USE, ONLY: you have "declare" the imported >>entities in every file that uses that module. > > So think of it as redeclaring the parts that you are about > to (ahem) use in client code. The USE, ONLY is about the > client code, not the module. > > >>Like I mentioned before, for the situation where there are many >>developers (e.g. 5-10+) working on various aspects of the same code >>I think there's a threshold beyond which it becomes onerous to list >>all the required imported stuff from a USEd module via an ONLY. > > > You'll only find out by trying it, won't you ? > > One thing, though: the payoff doesn't come when you write the > code. The payoff comes when you *read* the code (and also when > you modify it). > > If anything, having multiple developers should shift the threshold > *in favor of* USE, ONLY per routine. You impose added discipline > at the time of writing new code --which has a real cost-- in order > to simplify future development. You are absolutely 100% correct. I was going to reply earlier to James Giles' post pretty much backtracking some of the way, but your post says what I meant to say (much better than I would've phrased it, too). My beef was when legacy code needed to be updated (that happens) and the thought of constructing the monster USE, ONLYs seemed like a soul destroying task (I'm trying to find out how it was done here - by hand, or with some tool/script) > > >>Then again, maybe that's a good thing? Would it encourage people >>to write code that is broken down into smaller subsets of functionality >>rather than tossing a whole bunch of stuff into one monster(ish) >>module? > > Havent' you got this one backward ? USE, ONLY is even more important > with monster modules since it allows you to specify the subset of > the monster on which you actually depend. The small modules are > the ones that you could USE wholesale (if you don't expect them to > grow). Then again, if they are so small, why not add the ONLY > clause as a documentation aid to the client code ? Again, I agree (and you said it well.) I just find that when I do have things organised the way I like them, my "ONLY" list is more of an "ALL" list (if you know what I mean). But, again, I think the point that it aids future reading/modification is the best "selling point". > The only exception left is then the genuine monster module where > every use will refer to a sizeable portion of the interface. Where > that threshold lies I have no idea. Me neither, but I think a suitable tool should be used (similar to the tools that parse f90 source files and produce makefile dependency lists. Does one exist?) > >>And even then, will that spread the ONLY "declarations" >>/down/ the heirarchy or /across/ more, smaller modules > > So? USE statements in module procedures are a part of the > implementation, not the interface. They don't impact the design. > > Since this was about coding standards, here are a few thoughts. > 1) Nobody likes coding standards, but if they get over it > they'll get over it. I speak from experience. > 2) Just make sure the standards are not harmful. I think > *mandating* USE at module level would be harmful. > 3) You need a mechanism to grant waivers, because you > can't be sure your standards won't be harmful rather > than just annoying. > 4) You need a mechanism to amend the coding standards, > for the same reason. Excellent points all -- although I'm not sure I fully understand what you meant by #2. What do you mean by "mandating USE at module level"? Your post was extremely helpful - it has put the USE, ONLY debate to bed for me. Now all I have to do is tackle some of the other current requirements, specifically related to documentation, namely: - Document all changes, not only around modified code but also in the documentation block at the head of each subroutine or function. - When modifying a routine, remember to add a one-line entry with change date, programmer, and brief description. This one-line entry goes in the program history log of the documentation block. - Ensure input/output argument documentation in doc block is consistent with additions/deletions from calling list At one point I would have said "sure, fine, whatever" being, unfortunately, a master of the "Massive Function Header" (thanks to Bil Kleb for pointing out the wiki page containing that link) but now I think the above requirements are a bit much - especially once we get everyone using CVS (or Subversion) (no laughing ... it's been a trip to get it all setup.) Anyway, thanks again. Very useful post. cheers, paulv -- Paul van Delst CIMSS @ NOAA/NCEP/EMC |