[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Octave-patch-tracker] [patch #9057] Octave Mapping package: majaxis and
From: |
Philip Nienhuis |
Subject: |
[Octave-patch-tracker] [patch #9057] Octave Mapping package: majaxis and minaxis |
Date: |
Mon, 25 Jul 2016 07:27:55 +0000 (UTC) |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0 SeaMonkey/2.40 |
Follow-up Comment #3, patch #9057 (project octave):
Well, most of what you provided looks good, but there are things like this:
## Copyright (C) 2016
that rather should have a name:
## Copyright (2016) <your name>
and:
function a=majaxis(b, e)
## The equation is b=a*√(1-e^2)
## rearranging a=b/√(1-e^2)
a = b / sqrt(1 - e ^ 2);
endfunction
-verbatim
should look like:
function a = majaxis (b, e)
## The equation is b=a*√(1-e^2)
## rearranging a=b/√(1-e^2)
a = b / sqrt (1 - e ^ 2);
endfunction
and immediately after the deftypefn line there should be a concise one-liner
explaining what the function does w/o going into details. If on types:
help majaxis
that line is shown.
Also in the texinfo help text we'd like to have spaces around "=" to improve
readability. In comments that is less relevant although readability is also a
concern there.
There's a tiny section on the wiki about coding style, see:
http://wiki.octave.org/Octave_style_guide, but there must be more.
Or study core Octave functions to see the coding style.
Another important issue is error checks.
What happens if someone inputs e < 1? b < 1? b = 0? b and/or e non-numeric?
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/patch/?9057>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/