2

I'm a little bit confused on using If , Else if on my current project...I need to compare 6 values, the smallest one should be prompted by a message box..here's my code

if ((sort1 > sort2) || (sort1 > sort3) || (sort1 > sort4) || (sort1 > sort5))
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Insertion Sort with the time of " + elapsedMs1 + " ms");
}
else if ((sort2 > sort1) || (sort2 > sort3) || (sort2 > sort4) || (sort2 > sort5))
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Selection Sort with the time of " + elapsedMs2 + " ms");
}
else if ((sort3 > sort1) || (sort3 > sort2) || (sort3 > sort4) || (sort3 > sort5))
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Bubble Sort with the time of " + elapsedMs3 + " ms");
}
else if ((sort4 > sort1) || (sort4 > sort2) || (sort4 > sort3) || (sort4 > sort5))
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Merge Sort with the time of " + elapsedMs4 + " ms");
}
else if ((sort5 > sort1) || (sort5 > sort2) || (sort5 > sort3) || (sort5 > sort1))
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Quick Sort with the time of " + elapsedMs5 + " ms");
}

But it will not prompt the smallest number on the messagebox. Do this have a much proper coding rather than using if, else if? I've also tried using AND but it doesn't work too..

9
  • You can sort the results... Commented Feb 5, 2016 at 12:03
  • 2
    That's not C. C# perhaps? And what if none of the conditions are true, have you tried adding a single else at the end of the chain to see? Commented Feb 5, 2016 at 12:03
  • 1
    Off the cuff, I'd say replace your ||'s with &&'s... Commented Feb 5, 2016 at 12:04
  • what is sortX ? an int? Commented Feb 5, 2016 at 12:04
  • Better you use 'switch' statement to avoid confusion, if you want I can show you. First let me know what exactly you want. Commented Feb 5, 2016 at 12:05

6 Answers 6

1

Throw away this code and use a loop instead.

  • Put the results in an array.
  • Use a variable int largest_value = 0.
  • Use a variable int largest_index = -1.
  • Iterate through the array and for each result, check if it is larger than "largest_value".
  • If the value was larger, set largest_value = the value, and set largest_index = i;, where i is the loop iterator.

After the loop, largest_value will contain the largest value and largest_index will point out where in the array this value is found.

Sign up to request clarification or add additional context in comments.

Comments

1

I would store all the results in a dictionary like so (numbers are made up):

Dictionary<string, int>  myDictionary = new Dictionary<string, int>()
{
    {"Insertion Sort", 12},
    {"Selection Sort ", 35},
    {"Bubble Sort", 42},
    {"Merge Sort", 52},
    {"Quick Sort ", 32}
};

var min = myDictionary.First(kv => kv.Value == myDictionary.Values.Min());
Console.Out.WriteLine("The fastest is " + min.Key + " with a time of " + min.Value + "ms");

Which will allow you to test as many algorithm without changing your if/else structure (since you don't use it).

The output is:

The fastest is Insertion Sort with a time of 12ms

Comments

1

I suggest using Dictionary and Linq:

var algorithms = new Dictionary<String, Double>() { // or Dictionary<String, int>
  {"Insertion Sort", sort1},
  {"Selection Sort", sort2},
  ...
};

var best = algorithms
  .OrderBy(pair => pair.Value)
  .First();

MessageBox.Show(String.Format("The Best Sorting Technique Algorithm is {0} with the time of {1} ms", best.Key, best.Value));

The advantage of using Linq is that you can easily create a report you like, for instance let's print out algorothms from fastest to slowest:

var data = algorithms
  .OrderBy(pair => pair.Value)
  .Select(pair => String.Format("{0} took {1} ms", pair.Key, pair.Value));

MessageBox.Show(String.Join(Environment.NewLine, data));

Comments

0

It's not a matter with else if, but more with the operators you're using - a sorting algorithm is the fastest if it's faster than ALL others, not just one.

So, change the conditions to use the && operators instead of ||.

Please note, that this is not the best solution (see others' comments for that), it merely is a fox to your code.

Comments

0

I think you mean the following:)

if ( !( sort1 < sort2 ) && !( sort1 < sort3 ) && !( sort1 < sort4 ) && !( sort1 < sort5 ) )
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Insertion Sort with the time of " + elapsedMs1 + " ms");
}
else if ( !( sort2 < sort3 ) && !( sort2 < sort4 ) && !( sort2 < sort5 ) )
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Selection Sort with the time of " + elapsedMs2 + " ms");
}
else if ( !( sort3 < sort4 ) && !( sort3 < sort5 ) )
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Bubble Sort with the time of " + elapsedMs3 + " ms");
}
else if ( !( sort4 < sort5 ) )
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Merge Sort with the time of " + elapsedMs4 + " ms");
}
else 
{
    MessageBox.Show("The Best Sorting Technique Algorithm is Quick Sort with the time of " + elapsedMs5 + " ms");
}

I suppose that values of sort1, sort2 and etc can be equal each other. In this case the value with the lowest variable name is selected.:)

1 Comment

How to make an overcomplicated piece of code even less readable :)
0

Change every || to && you want all of them to be true.

Change every > to <, because you want the smallest, not the greatest.

You also have a typo in your last if: you have sort5 > sort1 twice and you don't have sort5>sort4

1 Comment

I just noticed you had > instead of <

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.