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

Merge sort C# Implementation

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

Problem

Its been years (10+) since I have worked with sorting algorithms directly. So I have been trying to go over them again to refresh my memory. The thing is my ideas of how to do it doesn't seam to match the way I have the tutorials doing it.

Does it matter? Assuming that my code is working I would almost mean that its correct. However I have also been looking into the Big-O I really cant remember ever seeing this before. I would almost think that if you code it incorrectly it will slow things down and mess up the performance.

```
public class MergeSort2
{

public MergeSort2()
{
var data = Util.CreateData.CreateRandomIntArray(200); // just a method that spits out an array of ints
var sortedData = Sort(data);

}

private int[] copyArray(int[] A, int start, int end)
{

int[] result = new int[end-start];

int cnt = 0;
for (int i = start; i Console.Write(a + ","));
Console.Write("\r\n");
Array.ForEach(LeftArray, a => Console.Write(a + ","));
Console.Write("\t - \t");
Array.ForEach(RightArray, a => Console.Write(a + ","));
Console.Write("\r\n");

RightArray = Sort(RightArray);
LeftArray = Sort(LeftArray);

var sorted = merge(RightArray, LeftArray);

Console.Write("Sorted: ");
Array.ForEach(sorted, a => Console.Write(a + ","));
Console.Write("\r\n");

return sorted;
}

public int[] merge(int[] right, int[] left)
{

int[] merged = new int[right.Length + left.Length];

int cntright = 0;
int cntleft = 0;

for (int i = 0; i < merged.Length; i++)
{
if (cntright == right.Length)
{
merged[i] = left[cntleft];
cntleft++;
}
else if (cntleft == left.

Solution

Inside of your copyArray you should not have the cnt variable separate from the for loop, you should just include it in the declaration. The name of that Method should be in PascalCase not in camelCase. You should also give operators some space (ie [end - start])

so this:

private int[] copyArray(int[] A, int start, int end)
    {

        int[] result = new int[end-start];

        int cnt = 0;
        for (int i = start; i < end; i++)
        {
            result[cnt] = A[i];
            cnt++;
        }

        return result;
    }


would become this

private int[] CopyArray(int[] A, int start, int end)
{
    int[] result = new int[end - start];
    for (int i = start, cnt = 0; i < end; i++, cnt++)
    {
        result[cnt] = A[i];
    }
    return result;
}


you can remove the cnt++ from inside the loop and just add it to the for loop declaration.

Variables should be camelCase

int[] LeftArray = copyArray(data,0,length);
 int[] RightArray = copyArray(data, length,data.Length);


These should be

int[] leftArray = CopyArray(data,0,length);
 int[] rightArray = CopyArray(data, length,data.Length);


And

public int[] merge(int[] right, int[] left)


should be

public int[] Merge(int[] right, int[] left)


Because Methods should always be in PascalCase

And

int cntright = 0;
int cntleft = 0;


should be

int cntRight = 0;
int cntLeft = 0;


for the same reason

Code Snippets

private int[] copyArray(int[] A, int start, int end)
    {

        int[] result = new int[end-start];

        int cnt = 0;
        for (int i = start; i < end; i++)
        {
            result[cnt] = A[i];
            cnt++;
        }

        return result;
    }
private int[] CopyArray(int[] A, int start, int end)
{
    int[] result = new int[end - start];
    for (int i = start, cnt = 0; i < end; i++, cnt++)
    {
        result[cnt] = A[i];
    }
    return result;
}
int[] LeftArray = copyArray(data,0,length);
 int[] RightArray = copyArray(data, length,data.Length);
int[] leftArray = CopyArray(data,0,length);
 int[] rightArray = CopyArray(data, length,data.Length);
public int[] merge(int[] right, int[] left)

Context

StackExchange Code Review Q#144439, answer score: 7

Revisions (0)

No revisions yet.