current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite 
if (progressBar1.Value == 1) { progressLabel.Text = "1%"; }
if (progressBar1.Value == 10) { progressLabel.Text = "10%"; }
if (progressBar1.Value == 20) { progressLabel.Text = "20%"; }
if (progressBar1.Value == 30) { progressLabel.Text = "30%"; }
if (progressBar1.Value == 40) { progressLabel.Text = "40%"; }
if (progressBar1.Value == 50) { progressLabel.Text = "50%"; }
if (progressBar1.Value == 60) { progressLabel.Text = "60%"; }
if (progressBar1.Value == 70) { progressLabel.Text = "70%"; }
if (progressBar1.Value == 80) { progressLabel.Text = "80%"; }
if (progressBar1.Value == 90) { progressLabel.Text = "90%"; }
if (progressBar1.Value == 100) { progressLabel.Text = "100%"; }
share|improve this question

closed as unclear what you're asking by Jamal♦ Jul 8 at 21:16

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.
    
I like dreza's answer, if you wanted to make this code more efficient you could use if..else if rather than several if statements. it would hit one, show it and exit the code until the next iteration of the code –  Malachi Sep 13 '13 at 14:43

3 Answers 3

up vote 16 down vote accepted

Why can't you do:

progressLabel.Text = string.Format("{0}%", progressBar1.Value)

As @MNZ pointed out I potentially didn't answer the question exactly how the OP asked. So here's a slightly modified alternative option:

progressLabel.Text = FormatProgress(progressBar1.Value, progressLabel.Text)

private string FormatProgress(int currentProgress, string progressDisplayed)
{
    if (currentProgress < 0 || currentProgress > 100)
        throw new ArgumentOutOfRangeException("Current progress is not in a valid percentage range of 0 to 100");

    if (currentProgress == 1) 
        return FormatProgress(currentProgress);

    return currentProgress % 10 == 0 ? 
              FormatProgress(currentProgress) : 
              progressDisplayed;
}

private string FormatProgress(int progress)
{
    return string.Format("{0}%", progress);
}

And a unit test to confirm:

[TestMethod]
public void Test()
{
    Assert.AreEqual("1%", FormatProgress(1, "0%"));
    Assert.AreEqual("0%", FormatProgress(0, "5%"));
    Assert.AreEqual("10%", FormatProgress(10, "6%"));
    Assert.AreEqual("80%", FormatProgress(85, "80%"));
    Assert.AreEqual("100%", FormatProgress(100, "90%"));
    Assert.AreNotEqual("75%", FormatProgress(75, "70%"));            
}
share|improve this answer
    
Although this code is not exactly same as the code in question. It must have an if satement to check if progressLabel.Value is one of 1,10,20,30,40,50,60,70,80,90 or 100. –  MNZ Sep 14 '13 at 0:16
    
+1 for the unit test! –  radarbob Sep 17 '13 at 20:47
up vote 1 down vote 
if (progressBar1.Value == 1 || progressBar.Value % 10 == 0)
    progressLabel.Text = progressBar1.Value.ToString() + "%";

Assuming that you are using a standard WinForms progress bar, you shouldn't have to worry about the progressBar.Value being <0 or >100.

share|improve this answer
up vote 1 down vote

Use a switch statement...

switch(progressBar.Value)
{
case 1:
progressLabel.Text = "1%"; 
break;

case 10:
progressLabel.Text = "10%"; 
break;

case 20:
progressLabel.Text = "20%"; 
break;

case 30:
progressLabel.Text = "30%"; 
break;
}

etc...

share|improve this answer
    
this would be more code than the if statements. it wouldn't be shorter. –  Malachi Sep 13 '13 at 14:41
    
@Malachi indeed, but it does address the multiple condition evaluation that you pointed out in your comment to the question, and is more elegant than a bunch of else blocks. –  Mat's Mug♦ Sep 13 '13 at 15:00
    
that is almost a toss up, but I think I am going to have to agree with you on that @retailcoder. –  Malachi Sep 13 '13 at 15:06
3  
Switch Statements are much more efficient than an else if. Else if statements force each if statement to be tested before finding the correct one. Case statements only test once. Plus it gives you more flexibility than drezas answer, although drezas IS much shorter. –  Nate S. Sep 13 '13 at 15:28
    
@CodeSlinger and that's what got you my upvote ;) ...although it is overkill in this specific case. –  Mat's Mug♦ Sep 13 '13 at 16:17

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