patternMinor
Adding and subtracting Matrices based on condition
Viewed 0 times
conditionaddingsubtractingbasedmatricesand
Problem
procedure E(n, m: integer; A, B: tMatrix; var C: tMatrix);
var i, j: integer;
begin
for i:=1 to n do
for j:=1 to m do begin
if i<=j then C[i,j]:=A[i,j] + B[i,j] else C[i,j]:=A[i,j] - B[i,j];
end;
end;Input: three matrix. Output: new matrix which created by addition or subtraction of two elements of two matrix depending on conditions
Solution
You are only using single-letter variable names. While this is quite understandable for
The big problem about
Speaking of names, your method is named
I believe it's convention in Delphi to use an uppercase letter for the
I'd use slightly more spacing in the code, and I'd also use a
This line:
is, except for the short and confusing variable names, not that bad really. I think it boils down to opinions how to write it 'the best'. An alternative to what you are doing is to use a
There is essentially a lot of different ways to write this. Use the way you like the best.
I suspect that the integer values passed to the procedure always match the size of the matrices, in that case, use rows and cols as properties of the matrices if possible.
A, B and C, I don't recommend it for n and m.The big problem about
n and m (and i and j) is that it is hard to tell what is the row and what is the column. (The thing I hate the most about mathematics is the undescriptive variable names, it is not something I'd personally recommend carrying over to the programming side of things)Speaking of names, your method is named
E. That really doesn't say anything whatsoever.I believe it's convention in Delphi to use an uppercase letter for the
T prefix, so that would be TMatrix.I'd use slightly more spacing in the code, and I'd also use a
begin also for the outer for-loop.This line:
if i<=j then C[i,j]:=A[i,j] + B[i,j] else C[i,j]:=A[i,j] - B[i,j];is, except for the short and confusing variable names, not that bad really. I think it boils down to opinions how to write it 'the best'. An alternative to what you are doing is to use a
addValue variable:procedure ConditionallyAdd(rows, cols: Integer; A, B: TMatrix; var C: TMatrix);
var
row, col: Integer;
addValue: Real; // or Integer? or something. The type that your matrix uses
begin
for row := 1 to rows do begin
for col := 1 to cols do begin
if row <= col then addValue = B[row, col];
else addValue = -B[row, col];
C[row, col] := addValue;
end;
end;
end;There is essentially a lot of different ways to write this. Use the way you like the best.
I suspect that the integer values passed to the procedure always match the size of the matrices, in that case, use rows and cols as properties of the matrices if possible.
Code Snippets
if i<=j then C[i,j]:=A[i,j] + B[i,j] else C[i,j]:=A[i,j] - B[i,j];procedure ConditionallyAdd(rows, cols: Integer; A, B: TMatrix; var C: TMatrix);
var
row, col: Integer;
addValue: Real; // or Integer? or something. The type that your matrix uses
begin
for row := 1 to rows do begin
for col := 1 to cols do begin
if row <= col then addValue = B[row, col];
else addValue = -B[row, col];
C[row, col] := addValue;
end;
end;
end;Context
StackExchange Code Review Q#90846, answer score: 3
Revisions (0)
No revisions yet.