Doug's MATLAB Video TutorialsAugust 21st, 2009Basics: Code review- the thought process in rewriting code for clarityThis short video covers the thought process of rewriting some code for clarity. The six lines of code are lengthened into twice that. The code is much more readable and maintainable. The problem is not as important as the thought process, however, the problem is about reshaping a matrix.
14 Responses to “Basics: Code review- the thought process in rewriting code for clarity”Leave a Reply |
|
|||
|
These postings are the author's and don't necessarily represent the opinions of The MathWorks. |
rowStart and indeed numSubMat should be variables that change depending on the numSubRows variable, otherwise if you change numSubRows in your code then it will no longer give the expected output
@Robert
aw! yeah you’re right bro.. thank you for this advice…
By the way, I notice there is an M-lint warning on line 54 complaining about the [x:y] syntax. Why is that considdered a warning? As your example shows, code usually ends up having them, and I see no reason why Matlab should have a problem with that.
–DA
@Robert,
The code would be better with that suggestion, slightly more complicated, but a good check as that is something that would likely be done wrong by coders working with the code later.
@Daniel,
The complaint on 54 is about using unneeded brackets on that line. The code runs just as well without them.
-Thanks
Doug
@dhull
Yes, but so are an extra pair of parenthesis, but sometimes you leave them in there anyways and you do not see matlab complaining. But that is a discussion to be had in a service request, not here.
Daniel Armyr,
I suspect the reason why MLint doesn’t warn about parenthesis is that they invoke no function call. Parenthesis are only used to group in this context. Brackets on the other hand, are used to concatenate. If there is only one array inside the brackets, there is nothing to concatenate, yet the internal concatenate function is called anyway! This is an inefficiency.
Hi, I’m new to this blog, and as a matter of fact I’m lost so I was kinda hoping someone will point to me the right link to post my questions about SimEvent tool box.
Thanks a lot in advance.
Ola,
http://www.mathworks.com/support can answer all your questions.
Thanks,
Doug
I’ve been using Matlab for a long time, but never thought of coding a test matrix like you did, e.g.,
1 2 3 4 …
11 12 13 14 …
.
.
.
Great idea for checking how code manipulates data, in stages
Hi,
I’d think that all the original code needs is a replacement of “shiftdim” with “permute”. That gets you rid of the “shiftdata” function, since shiftdim can only circulate dimensions; permute is more general:
I like the idea of expansion into higher dimensions, I use it a lot. Loops are fine, but can be tedious for high-dimensional problems. If one prints out b2 (; left out), it becomes quite clear that the result is there already — all that needs to be done is reshuffling of dimensions for the next reshape. Also such code (hopefully) operates on indexes, rather than data and checks the dimensions for you, to some extend.
I would normally do this on one line to save memory. There are pros and cons, of course. When the array becomes too large, one might think of segmenting it and saving intermediate results within a loop.
Thanks,
Ludvik
@Ludvik,
Thank you for your comments. You make good points, there are trade-offs. I tend to work on smaller datasets, and am mostly concerned about readability. It sounds like you have other metrics to satisfy, so you use other techniques.
Thanks,
Doug
For many years the Mathworks promoted the idea of eliminating loops (vectorization) whenever possible, both for efficiency and also because in many cases the matrix operations were clearer. For an extreme example it is much easier to see what B = A’ means then to interpret an equivalent set of nested loops.
Yes I agree that brevity isn’t the end all, but I think that your use of loops here where they really aren’t needed is too extreme and tends to miss the point of using a powerful matrix based language rather than coding in a more primitive language where everything has to be done in loops.
I would propose the following alternative code:
function B = interleavedTranspose(A) %INTERLEAVEDTRANSPOSE Interleaves alternate rows of input matrix and %transposes to produce output matrix % % we wish to rearrange the input matrix as follows: % 1) interleave every other row with the row above it % 2) transpose the result % So for example if we start with the matrix % [11 12 13 ; % 21 22 23; % 31 32 33; % 41 42 43] % would first interleave every other row with the row above it to get % % [11 21 12 22 13 23; % 31 41 32 42 33 43] % % and then transpose to get: % % [11 31; % 21 41; % 12 32; % 22 42; % 13 33; % 23 43] % % We will do this by first separating alternate rows (odd and even row % numbers into two matrices % oddRows = [11 12 13 ; % 31 32 33] % evenRows = [21 22 23; % 41 42 43] % then make a new matrix to hold the result % B = [ 0 0 0 0 0 0 ; % 0 0 0 0 0 0 ] % % place the odd rows into new matrix leaving spaces to interleave even rows % B = [ 11 0 12 0 13 0; % 31 0 32 0 33 0] % interleave the even rows % B = [ 11 21 12 22 13 23 ; % 31 41 32 42 33 43] % transpose % B = B' % input matrix must have even number of rows % get dimensions of input matrix for later use [numRows,numCols] = size(A); % first check that input matrix has even number of rows if rem(numRows,2)~= 0 error('input matrix must have even number of rows') end % separate alternate rows and put into two matrices oddRows = A(1:2:numRows,:); evenRows = A(2:2:numRows,:); % make new matrix to hold interleaved rows B = zeros(numRows/2,numCols*2) % place odd rows into new matrix leaving spaces to interleave even rows B(:,1:2:end) = oddRows; % interleave even rows B(:,2:2:end) = evenRows; % transpose to get final result B = B' end@Jon,
That looks well documented and formatted!
Thanks
Doug