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

Better implementation of Gaussian Elimination

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationeliminationbettergaussian

Problem

I made an algorithm in C# that solves any system of linear equations using the Gaussian elimination. There are 2 text boxes in the program for input and output. Input is in the format of the coefficients of the variables separated by spaces and lines. I want to know if this code can be cut shorter or optimized somehow.

```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Equations_Solver
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
}

private void button1_Click(object sender, EventArgs e)
{
textBox2.Clear();
double[][] rows = new double[textBox1.Lines.Length][];
for (int i = 0; i i; z--)
{
if (rows[z][i] != 0)
{
double[] temp = new double[length];
temp = rows[z];
rows[z] = rows[j];
rows[j] = temp;
changed = true;
}
}
if (!changed)
{
textBox2.Text += "No Solution\r\n";
return;
}
}
if (rows[j][i] != 0)
{
d[x] = rows[j][x] / rows[j][i];
}
else
{
d[x] = rows[j][x];
}
}
rows[j] = d;
}
for (int y = i + 1; y =

Solution

If you concatenate strings inside a loop it is much better to use a stringbuilder.

Instead of placing the whole code in a button click handler and accessing the textboxes, you should give this code an own method which is then called in the handler.

This if condition

for (int x = 0; x < length; x++)
{
    if (i == j && rows[j][i] == 0)
    {


can only be true for the first iteration so if you extract the swapping of the array elements to a separate method which is called before this loop, you won't need to check this for every iteration.

if (rows[j][i] != 0)
{
    d[x] = rows[j][x] / rows[j][i];
}
else
{
    d[x] = rows[j][x];
}


By just setting d[x] = rows[j][x] before the if statement you could reduce this to

d[x] = rows[j][x];
if (rows[j][i] != 0)
{
    d[x] =  d[x] / rows[j][i];
}


The same should be done for

if (rows[y][i] != 0)
{
    f[g] = rows[y][g] - rows[i][g];
}
else
{
    f[g] = rows[y][g];
}


This

double val = 0;
int k = length - 2;
double[] result = new double[rows.Length];
for (int i = rows.Length - 1; i >= 0; i--)
{
    val = rows[i][length - 1];
    for (int x = length - 2; x > k; x--)
    {
        val -= rows[i][x] * result[x];
    }
    result[i] = val / rows[i][i];
    if (result[i].ToString() == "NaN" || result[i].ToString().Contains("Infinity"))
    {
        textBox2.Text += "No Solution Found!\n";
        return;
    }
    k--;
}


should be extracted to its own method and should be simplified by removing k and change the inner loop to

for (int x = length - 2; x > i - 1; x--)


By extracting the splitting and converting of the string[] to a separate method, you gain in readability and separation of concerns.

Applying the mention points will lead to

private double[] SolveLinearEquations(string[] input)
{
    double[][] rows = new double[input.Length][];
    for (int i = 0; i  row; z--)
    {
        if (rows[z][row] != 0)
        {
            double[] temp = new double[rows[0].Length];
            temp = rows[z];
            rows[z] = rows[column];
            rows[column] = temp;
            swapped = true;
        }
    }

    return swapped;
}
private double[] CalculateResult(double[][] rows)
{
    double val = 0;
    int length = rows[0].Length;
    double[] result = new double[rows.Length];
    for (int i = rows.Length - 1; i >= 0; i--)
    {
        val = rows[i][length - 1];
        for (int x = length - 2; x > i - 1; x--)
        {
            val -= rows[i][x] * result[x];
        }
        result[i] = val / rows[i][i];

        if (!IsValidResult(result[i]))
        {
            return null;
        }
    }
    return result;
}

private bool IsValidResult(double result)
{
    return !(double.IsNaN(result) || double.IsInfinity(result));
}


which can then be called like

double[] result = SolveLinearEquations(textBox1.Lines);  

textBox2.Clear();

textBox2.Text = ConvertToString(result);


where ConvertToString() will look like

private string ConvertToString(double[] result)
{
    StringBuilder sb = new StringBuilder(1024);
    for (int i = 0; i < result.Length; i++)
    {
        sb.AppendFormat("X{0} = {1}\r\n", i + 1, Math.Round(result[i], 10));
    }
    return sb.ToString();
}

Code Snippets

for (int x = 0; x < length; x++)
{
    if (i == j && rows[j][i] == 0)
    {
if (rows[j][i] != 0)
{
    d[x] = rows[j][x] / rows[j][i];
}
else
{
    d[x] = rows[j][x];
}
d[x] = rows[j][x];
if (rows[j][i] != 0)
{
    d[x] =  d[x] / rows[j][i];
}
if (rows[y][i] != 0)
{
    f[g] = rows[y][g] - rows[i][g];
}
else
{
    f[g] = rows[y][g];
}
double val = 0;
int k = length - 2;
double[] result = new double[rows.Length];
for (int i = rows.Length - 1; i >= 0; i--)
{
    val = rows[i][length - 1];
    for (int x = length - 2; x > k; x--)
    {
        val -= rows[i][x] * result[x];
    }
    result[i] = val / rows[i][i];
    if (result[i].ToString() == "NaN" || result[i].ToString().Contains("Infinity"))
    {
        textBox2.Text += "No Solution Found!\n";
        return;
    }
    k--;
}

Context

StackExchange Code Review Q#79462, answer score: 6

Revisions (0)

No revisions yet.