[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: savevtk

From: Philip Nienhuis
Subject: Re: savevtk
Date: Wed, 17 Nov 2010 22:53:58 +0100
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20101027 SeaMonkey/2.0.10

Levente Torok wrote:
Dear Philip, Carlo and octave-dev,

I had a chance to polish things according to your commands.

Thanks, I'll have a look at them.

However I have a few questions.

Many of the codes do not fit into this scheme but I can accept that
this is guideline for the future.

- 2 spaces rather than tabs (but I still use tabs in my own scripts so I
don't mind this particular one)

- spaces between internal function name and left paren; no space between
array name and left paren or bracket

- functions end with "endfunction" rather than "return"

- appropriate end statements: if- elseif -else - endif / for - endfor /
while - endwhile / etc.

Why dont want we write code that maybe used with matlab too?
This would be a benefit for all I believe.

O sure, I believe that too, OTOH there are good reasons to keep Octave style. For one, and also my main motive: Octave's script language is simply superior to that of ML. (It's easy to write e.g., if...end%if and for...end%for to retain ML compatibility but Octave's parser can more reliably check code if we follow to the above style suggestions.) Other -probably more ideological- reasons have been brought up repeatedly by Octave developers in the help-octave and octave-maintainers mailing lists.

From a more practical point of view, code that is meant to be run unchanged under ML doesn't seem to belong in the io package in the "main" tree, but rather in some package in the "extra" tree.
See here:
section "Where does your code belong?"

Unless explicitly instructed otherwise with sufficiently convincing reasoning I tend to stick to these guidelines. (Though I violated this already by provisionally committing a first versions of your scripts to svn, assuming updated and more compliant versions will follow.)

- spaces after commas in argument lists (e.g.,  "(a, b, c)" instead of

- are you sure your function shouldn't return some value, for successful
writing or not? I see no write error catching structures at all

- I think you should check for the return value of fopen and perhaps also
fprintf to see if the calls succeeded (hint: fprintf returns the number of
characters written).
A try-catch or unwind_protect construct could be around the fprintf
statements to maybe avoid dangling file pointers. Upon errors and ensuing
break out of savevtk....() the output file will probably be closed anyway
but personally I like more graceful solutions.

- comments start with # rather than %
Why is this if we support multiple type of commenting styles?

See above.

- in the Updates: section in the headers, please add some short description
of what the update was all about

- The copyright sections in the headers seem incomplete

- in savevtk.m an empty line is needed between the copyright statement and
the texinfo header

- the texinfo headers seem a bit odd to me. Have a look at e.g. the Excel or
OpenOffice.org scripts for examples

Where are these?

In the io package. But if you use the octave-3.2.4 MingW binary for Windows, it is easier to just do from Octave:
  edit newfile.m
and you'll probably have a nice template.

- would a test case or some test section be useful?
I don't know the syntax of the example however it could be something like:

for x=1:n
   for y=1:n
     for z=1:n
       X(x,y,z)=1/sqrt( x*x + y*y + z*z );
Maybe someone can write the same code with matrix arithmetic I did
tackle with this.

But OK, maybe I am just too picky....

According to Carlo, to some extent I am.... :-) (see his last mail/post to me)

If you want your scripts in the io package I could adapt the code (-style) a bit further for you (and send them to you for review before committing them), but if you want to retain ML compatibility I suppose your scripts perhaps should rather be elsewhere in the svn trunk.

Let me know what you want.

Thank you Levente.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]