I'm currently working in the math library for a game framework that I'm writing and am working to improve some functionality that was affected during a refactor.
Formerly, my Matrix4.SwapColumns(int, int)
function looked like this:
/// Documentation
public void SwapCols(int i, int j)
{
for(int k = 0; k < 4; k++)
{
temp = this[k][i];
this[k][i] = this[k][j];
this[k][j] = temp;*/
}
}
This worked because Matrix4[x][y]
was a two-dimensional float
array, which could be accessed and changed in one step. Now, since I'm using a new version of the operator as follows:
public Vector4 this[int a]
{
get
{
if (a > 3 || a < 0) return Vector4.Zero;
return (a == 0 ? row1 : (a == 1 ? row2 : (a == 2 ? row3 : row4)));
}
set
{
if (a == 0) row1 = value;
else if (a == 1) row2 = value;
else if (a == 2) row3 = value;
else if (a == 3) row4 = value;
}
}
The original method is no longer possible since the values that are part of the Vector4
are immutable, due to a C# language feature relating to properties (which I usually love having, since it makes sure you don't think you're reassigning the variable in one call to the properties' actual method implementations).
Here's the code review I need. Essentially, the new SwapColumns
method looks like this:
/// Documentation
public void SwapCols(int i, int j)
{
// New arguement checking goes here
int[] indicies = new int[] { 0, 1, 2, 3 };
indicies[i] = j;
indicies[j] = i;
Vector4 tempRow = Vector4.Zero;
int[] positions = new int[4];
for (int row = 0; row < 4; row++)
{
tempRow = this[row];
for (int col = 0; col < 4; col++)
{
if (indicies[col] != col)
{
tempRow[col] = this[row][indicies[col]];
}
}
this[row] = tempRow;
}
}
Without changing the fact that my Matrix4[]
returns a Vector4
, since that API needs to stay the same, how can I improve this SwapCols
method?