Mon 18 Jan 2016 02:41:31 AM UTC, comment #4:
Thanks Luis. I think I understand now.
This code looks very clean.
To increase its chances of getting included in Octave, follow the guidelines at https://www.gnu.org/software/octave/doc/interpreter/General-Guidelines.html. I'd recommend the following changes:
0. Make this a drop-in replacement for nchoosek. In particular, it must handle the (usual) case that the first argument is a scalar, in which it should return the binomial coefficient.
1. Include a GNU licence at the start, listing you as the copyright holder. See existing Octave .m files for a template
2. Validate the input before using it. What if this is called with only one input argument? If it is called with 3 it will work, but should report an error. Follow the example text when you give too many arguments to existing Octave functions.
3. Put a space before () that are a function call, but keep the () and {} for a variable reference with no space, as you have now.
4. Add a documentation string to the start, so that "help nchoosek" describes the new functionality. Look at some existing .m files for the (texi?) markup.
5. Check if there is much performance penalty for the (usual?) case that the input is numeric. If so, you may want to have an "if" use the builtin nchoosek directly for arguments that it handles. It may be best to combine this with step 0.
Thanks in advance for your effort. I hope that this makes it into a future release.
|