[Octave-patch-tracker] [patch #8060] [image package new function] whitepoint.m

[Carnë Draug]

Update of patch #8060 (project octave):

             Assigned to:                    None => carandraug             

    _______________________________________________________

Follow-up Comment #2:

Hi!

Aside the case insensitivity issue, which can be solved with using


  switch (tolower (string))


there's a few other issues:

1) no documentation. Could you please write documentation in TexInfo. See
other functions in Octave for an example. It's pretty simple.
2) your function only has 4 digit precision, but there's an actual formula
which can give a much correct value. See
http://en.wikipedia.org/wiki/Standard_illuminant 
3) no input check. For this case, the function should start with:


function xyz = whitepoint (standard = "icc")
  if (nargin > 1)
    print_usage ();
  elseif (! ischar (standard))
    error ("whitepoint: STANDARD must be a string.");
  endif 


Note on how we are also setting the default value right at the start.

In addition, other things that are just the guidelines for contributions

4) the main function block should also be indented
5) use double quote for strings unless (there's a really good reason
otherwise)
6) error message must mention the function name, the variable name (in caps)
as it's named in the documentation (you don't have documentation yet so I'm
calling it standard), and the wrong value that was given. So it should be
something like:


  error ("whitepoint: unknown STANDARD `%s'.", standard);


Note that if you made input check as I mentioned above, you'll know it's a
string, so you can use %s.

7) specify what block is ending with endfunction, endswitch, etc...

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8060>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/