HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Adding and subtracting Matrices based on condition

Submitted by: @import:stackexchange-codereview··
0
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 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.