This function takes in two matrices P and D, presumably the output
from Matlab's eig function, and then sorts the columns of P to
match the sorted columns of D (going from largest to smallest)

Although this function may appear "trivial" to some, I found it very useful. Since I don't rely on the help text, I read the code to see what it does before using it. Thus, this script was useful and sufficiently commented. Thanks for writing it and saving me time.

25 Feb 2008

Duane Hanselman

Please revise the help text to match standard MATLAB help text formating. This function would be useful if it gave the user more control over sorting, e.g., ascending or descending, or like that provided in esort (Control Toolbox) or cplzpair (basic MATLAB). For example, [p,idx]=esort(diag(D)); D=diag(p); P=P(:,idx); is much more useful.

25 Feb 2008

John D'Errico

This is not bad code. Its merely not terribly good. It even does what it says it does, despite the triviality of that action.

What could the author have done better?

The help is barely acceptable. Excellent help would clearly specify the expected shapes and sizes of the inputs. It would explain what the variables are, not make us guess that D is the diagonal array of eigenvalues, P the array of eigenvectors. Instead, this code merely refers to the output of eig. The funny thing is, if we look at the help for eig, it calls the variables V and D. Yeah, I know that many texts use P and D for the eigenvectors and diagonal matrix of eigenvalues, but I'm sorry, this is not what I choose to call good.

Next, you are expected to infer how to use this code from a single poor example. The author apparently thinks we will know that D is an array of eigenvalues, since it is diagonal. I suppose that might make some sense. But a diagonal matrix can as easily be a set of linear independent vectors. If the example is to be a clear one, I'd have expected P to be a set of independent vectors. NOT true here.

If you write code for others to use, make it good. Make it useful. If its trivial code, at least document it well.

Next, good code would have error checks. What happens if the user screws up and passes in a VECTOR of eigenvalues instead of an array? This seems like an easy mistake one might make. Check the sizes of your arguments. Verify that they make sense. Don't just let Matlab respond with some random, unintelligent error. Help your user.

Also, good code will have an H1 line. An H1 line is the FIRST line of the help block. It is used by lookfor to search on for keywords, so that the user can find some piece of code they dug up last year with a name they cannot recall. What was the first line of help in this submission?

% this function takes in two matrices P and D, presumably the output

A better H1 line might have looked like

% Sort eigenvalues and corresponding eigenvectors

Part of me wanted to rate this as a 2 ("needs improvement"). Why did I choose a 3 rating? While the code really is trivial, it does have some help and an example. It even has a readable (and useful) line of internal comment for each line of code in the submission.