5
\$\begingroup\$

I'm a beginner and like to experiment new things so today I tried to implement sorting method for an array without using any Sort and the actual challenge was to do it without using the bubble sort algorithm. I created my own but I feel that there are some things that can be shorten because it's quite long. My main idea was to work with indexes. Depending on how much numbers are bigger than the current one the index will increase with 1.

internal class Program
{
    private static void Main()
    {
        Console.ForegroundColor = ConsoleColor.White;
        const string exitCommand = @"/exit";
        Console.WriteLine(
            @"You can exit at any time by typing {0} or {1} this command will also stop you from entering more numbers and sort the current ones",
            exitCommand, exitCommand.Substring(1));
        while (true)
        {
            try
            {
                Console.Write("Enter Length : ");
                string inputLength = Console.ReadLine();
                if (inputLength != null && (inputLength.ToLower() == exitCommand || inputLength.ToLower() == exitCommand.Substring(1))) break;
                if (inputLength != null)
                {
                    int length = int.Parse(inputLength);
                    int[] testArray = new int[length];
                    for (int i = 0; i < testArray.Length; i++)
                    {
                        Console.Write("Enter number {0} : ", i + 1);
                        string input = Console.ReadLine();
                        if (input != null && (input.ToLower() == exitCommand || input.ToLower() == exitCommand.Substring(1))) break;
                        if (input != null) testArray[i] = int.Parse(input);
                    }
                    testArray = SortArray(testArray);
                    Console.WriteLine();
                    Console.ForegroundColor = ConsoleColor.Green;
                    Console.Write("Sorted Sequence : ");
                    foreach (var i in testArray)
                    {
                        Console.Write(i + " ");
                    }
                }
                Console.ForegroundColor = ConsoleColor.White;
            }
            catch (Exception ex)
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine(ex.Message);
                Console.ForegroundColor = ConsoleColor.White;
            }
            Console.WriteLine();
        }
    }

    private static int[] SortArray(IReadOnlyCollection<int> inputArray)
    {
        int[] newArray = new int[inputArray.Count];
        for (int i = 0; i < newArray.Length; i++)
        {
            newArray[i] = int.MinValue;
        }
        foreach (int t1 in inputArray)
        {
            var countRepeated = CountRepeated(newArray, t1);
            int count = inputArray.Count(t => t1 > t);
            int tempIndex;
            var isContained = IsContained(newArray, t1, out tempIndex);
            if (isContained)
            {
                newArray[tempIndex + countRepeated] = t1;
            }
            else
            {
                if (IsLower(newArray, count))
                {
                    newArray[CheckLower(newArray, count)] = t1;
                }
                else if (IsHigher(newArray, count))
                {
                    newArray[CheckHigher(newArray, count)] = t1;
                }
                else
                {
                    newArray[count] = t1;
                }
            }
        }
        return newArray.ToArray();
    }

    private static int CountRepeated(IReadOnlyList<int> newArray,int valueToCheck)
    {
        int countRepeated = 0;
        for (int i = 0; i < newArray.Count - 1; i++)
        {
            if (newArray[i] == valueToCheck)
            {
                countRepeated++;
            }
        }
        return countRepeated;
    }

    private static bool IsContained(IReadOnlyList<int> newArray, int t1, out int tempIndex)
    {
        bool isContained = false;
        tempIndex = 0;
        for (int index = 0; index < newArray.Count; index++)
        {
            var t = newArray[index];
            if (t != t1) continue;
            isContained = true;
            tempIndex = index;
            break;
        }
        return isContained;
    }

    private static bool IsHigher(IReadOnlyList<int> inputArray, int newIndex)
    {
        int[] tempIndex = {newIndex};
        return inputArray.Any(t1 => inputArray.Any(t2 => inputArray[tempIndex[0]] < t2));
    }

    private static int CheckHigher(IReadOnlyList<int> inputArray, int newIndex)
    {
        int[] tempIndex = {newIndex};
        foreach (
            int t2 in
                from t2 in inputArray
                from t1 in (from t3 in inputArray where inputArray[tempIndex[0]] > t3 select t2)
                select t2)
            tempIndex[0]++;
        return tempIndex[0];
    }

    private static bool IsLower(IReadOnlyList<int> inputArray, int newIndex)
    {
        int[] tempIndex = {newIndex};
        return inputArray.Any(t1 => inputArray.Any(t2 => inputArray[tempIndex[0]] < t2));
    }

    private static int CheckLower(IReadOnlyList<int> inputArray, int newIndex)
    {
        int[] tempIndex = {newIndex};
        foreach (int t4 in from t4 in inputArray
            from t2 in (from t3 in inputArray where inputArray[tempIndex[0]] > t3 select t4 into t1 select t4)
            select t4)
            tempIndex[0]--;
        return tempIndex[0];
    }
}
\$\endgroup\$
  • \$\begingroup\$ CheckLower and CheckHigher are hard to understand since the foreach has such a complex query (???), what is the point of it? can you please explain it so I can be of help, also some kind of comments on what each code block intends to do might be helpful for us. \$\endgroup\$ – Oscar Guillamon Mar 31 '16 at 10:36
  • 1
    \$\begingroup\$ I'd recommend trying to do some complexity analysis on your algorithm. You're looping over the arrays a lot. Also, because it's relevant: the most amazing sorting visualisations you'll ever see (probably) bost.ocks.org/mike/algorithms/#sorting \$\endgroup\$ – RobH Mar 31 '16 at 11:33
2
\$\begingroup\$
if (inputLength != null && (inputLength.ToLower() == exitCommand || inputLength.ToLower() == exitCommand.Substring(1))) break;

This line is both complex and repeated. Perhaps you can make it a function instead?

bool ShouldBreakOnLine(const string)

Apart from that, I also don't like seeing really long and complex lines. I suggest you split it into multiple lines on the && and || to improve readability.

On the other hand, I don't think you need the user to input the length at all. Instead of using an array with a fixed size, use List. It will automatically resize as you add new items to the end of it with the Add method.

Now, if you feel confident enough to implement your own sorting algorithm, I think you should also try implementing one of the famous ones. I recommend starting with Insertion sort, which is one of the simpler ones.

As for the rest of the code, I don't think I'm proficient enough in C# to review it. However, I can say this: as a reader who doesn't really understand the code, the difference between IsHigher and CheckHigher is not obvious. I'm not even sure what they do at all. Try using more descriptive names and write comments to explain things that still are hard to understand for an outsider (or you in a couple of months).

\$\endgroup\$
  • \$\begingroup\$ Thanks you for the answer jacwah, I made the long if statement into a function now, however can you suggest anything else in the other methods aside from static void Main or something that you don't like there ? \$\endgroup\$ – Denis Mar 31 '16 at 9:37
  • \$\begingroup\$ @denis I updated my answer :) \$\endgroup\$ – jacwah Mar 31 '16 at 9:57
  • \$\begingroup\$ Wow I'm completely ashamed of myself those methods happen's to be useless.. Just because you said they don't make good sense I double checked them and saw that it can go without them. \$\endgroup\$ – Denis Mar 31 '16 at 19:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.