Kian
Kian

Reputation: 89

Don't know why my loop does not loop

I'm new at programming and have been working on a program that converts a saunas temperature from Fahrenheit to Celsius and then tells the user if he/she should turn the heat up or down depending on the original input number. I've done the majority of my code but now I can't see why it does not loop when I write a number below 73 degrees or over 77 degrees. Can anyone see the problem that my eyes seem to not find?

using System;

namespace BastunKP
{
    class Program
    {
        public static int FahrToCels(int fahr)
        {
            int tempCels = (fahr - 32) * 5 / 9;
            return tempCels;
        }

        public static void Main(string[] args)
        {
            Console.WriteLine("Skriv in Fahrenheit: ");
            int fahr = int.Parse(Console.ReadLine());
            int tempCels = FahrToCels(fahr); 

            do
            {
                if (tempCels < 73)
                {
                    Console.WriteLine("Temperaturen är för kallt, skruva upp lite!");
                }
                else if (tempCels > 77)
                {
                    Console.WriteLine("Temperaturen är för varmt, skruva ner lite!");
                }
                else
                {
                    Console.WriteLine("Temperaturen är nu lagom, hoppa in!");
                    return;
                }

                fahr = int.Parse(Console.ReadLine());
                tempCels = FahrToCels(fahr);
            }           
            while (tempCels < 73 && tempCels > 77);
        }
    }
}

I also have a question regarding my assignment where the teacher has said that for a higher grade I should look into where the formula for converting fahrenheit to celsius and make it a double but I dont know how to do this change at all.

Thanks in advance

Upvotes: 3

Views: 153

Answers (4)

Kamil Solecki
Kamil Solecki

Reputation: 1117

Welcome to StackOverflow! Now, let's get down to your question:

First off, consider your do-while loop.

do {
    if (tempCels < 73) {
        // Temperature too high
        Console.WriteLine("Temperaturen är för kallt, skruva upp lite!");
    } else if (tempCels > 77) {
        // Temperature too low
        Console.WriteLine("Temperaturen är för varmt, skruva ner lite!");
    } else {
        // Temperature just right, hop in!
        Console.WriteLine("Temperaturen är nu lagom, hoppa in!");
        return;
    }

    fahr = int.Parse(Console.ReadLine());
    tempCels = FahrToCels(fahr);

}
while (tempCels < 73 || tempCels > 77);

As you can see, I removed the unnecessary else condition. What happens right now, is that all possible conditions are checked (temp < 73, temp > 77, and 73 < temp < 77).

One mistake you had, also pointed out in other answers, is that you had && (AND) instead of || (OR). And of course, a value cannot be both under 73 and above 77 :)

Now, I'd like to also point out some styling / general things I think you should 'fix':

1) Your temp conversion method contains an unnecessary variable creation and assignment. You can make it work just as well without it, like this:

    public static int fahrToCels(int fahr) {
        // It returns just the same, without needing to create a new, 
        // temporary temperature variable! 
        return (fahr - 32) * 5 / 9;
    }

2) This might be debatable, but general naming conventions say that function names are written with camelCase.

3) While this is not a problem in your scenario specifically, it might become one when you scale up an application (or work on a bigger one). It's best to use slightly more descriptive namings (in a bigger project, just fahr might be confusing). Again, it's not any big deal of a problem, just something for you to consider for the future :)

P.S. I did not change variable names in my examples, just to keep it more readable/relateable to the code you showed.

EDIT:

As per request, here is how to keep the values as double type.

    // Notice the return type and the property types are both double.
    public static double fahrToCels(double fahr) { 
        return (fahr - 32) * 5 / 9;
    }

This way, values don't have to be only integers, and produce weird results on division - they can be of type double too!

Now, remember you will need to pass a variable of type double to the function, otherwise you will get a type error.

Hint:

double fahr = int.Parse(Console.ReadLine());

Will let the user pass a non-integer value (like, say, 17.7), and it will be stored properly.

Hint #2:

If you really want to do on the fly conversion, you can achieve this like this (example values):

int ourInteger = 4;
double ourNewDoubleNumber = (double)ourInteger / 23;

You can read more about types and type casting here: Types and Type Casting

Upvotes: 2

unalignedmemoryaccess
unalignedmemoryaccess

Reputation: 7441

tempCels < 73 && tempCels > 77 is never true!

Most probably you wanted || so to run when temp is less than 73 or greater than 77, but who knows.

Upvotes: 4

Mureinik
Mureinik

Reputation: 311338

tempCels (or any number, for that matter) can't be less than 73 and more than 77 at the same time. You should use the logical || operator, not the logical && operator:

do {
    // code
} while (tempCels < 73 || tempCels > 77);
// Here ---------------^

Upvotes: 2

HappyCactus
HappyCactus

Reputation: 2014

while will loop when the condition is true, but tempCels cannot be <73 and >77 at the same time! Fix that condition and it will work.

Upvotes: 0

Related Questions