Search code examples
c#stringtype-conversionintegerlogic

cant seem to get anything but garbage value


Code to get the input in sting in form of csv("23,25,27) and then find the highest value of them 27(type int) all. But q seem to return garbage value everytime.

static void Main(string[] args)
{
        var p=0;`enter code here`
        int q=0,r,s=0;
        Console.WriteLine("Enter the numbers seperated by comma");
        var input = Console.ReadLine();
        for (var i = 0; i < input.Length; i++)
        {
            if (input[i] == ',')
            {
                p = 0;
                s = (s > q) ? s : q;
            }
            //else if (input[i] == ' ')
            //{
            //    p = 0;
            //    continue;
            //}
            else
            {
                q = (p == 1) ? q * 10 + Convert.ToInt32(input[i]) : Convert.ToInt32(input[i]);
                Console.WriteLine(q);
                p = 1;
            }
        
        }
        
        Console.WriteLine(s);
    }

OUTPUT: Enter the numbers seperated by comma 23,27 50 551 50 555 551


Solution

  • Rather than using p, q, s, let's rename those variables to hasSeenAtLeastOneDigit, parsed, and currentHighest respectively, which are much more descriptive names. Also note that hasSeenAtLeastOneDigit should be a bool, and r isn't used anywhere.

    var hasSeenAtLeastOneDigit = false;
    int parsed=0,currentHighest = 0;
    Console.WriteLine("Enter the numbers seperated by comma");
    var input = Console.ReadLine();
    for (var i = 0; i < input.Length; i++)
    {
        if (input[i] == ',')
        {
            hasSeenAtLeastOneDigit = false;
            currentHighest = (currentHighest > parsed) ? currentHighest : parsed;
        }
        else
        {
            parsed = hasSeenAtLeastOneDigit ? parsed * 10 + Convert.ToInt32(input[i]) : Convert.ToInt32(input[i]);
            // Console.WriteLine(parsed);
            hasSeenAtLeastOneDigit = true;
        }
    
    }
    
    Console.WriteLine(currentHighest);
    

    You have 2 mistakes here. First, you have accidentally used this overload of ToInt32 that takes a char, which converts a char to its UTF-16 code unit value, not the digit that it represents. You should have used the overload that takes a string or int.TryParse if you want to be safer.

    Convert.ToInt32(input[i].ToString())
    //                      ^^^^^^^^^^^
    //         converts to string, so the string overload is used
    

    The second mistake is that you are not comparing the last parsed value to currentHighest. Consider the case where input is 1,2,3,4. After the loop ends, parsed would be 4 and currentHighest would be 3 which is incorrect. This happens because there is no trailing comma at the end to run this line again:

    currentHighest = (currentHighest > parsed) ? currentHighest : parsed;
    

    You can fix this simply by copying and pasting that line at the end of the loop.

    var hasSeenAtLeastOneDigit=false;
    int parsed=0,currentHighest=0;
    Console.WriteLine("Enter the numbers seperated by comma");
    var input = Console.ReadLine();
    for (var i = 0; i < input.Length; i++)
    {
        if (input[i] == ',')
        {
            hasSeenAtLeastOneDigit = false;
            currentHighest = (currentHighest > parsed) ? currentHighest : parsed;
        }
        else
        {
            parsed = hasSeenAtLeastOneDigit ? parsed * 10 + Convert.ToInt32(input[i].ToString()) : Convert.ToInt32(input[i].ToString());
            hasSeenAtLeastOneDigit = true;
        }
    
    }
    currentHighest = (currentHighest > parsed) ? currentHighest : parsed;
    Console.WriteLine(currentHighest);
    

    Also note that the entire algorithm can be done with LINQ like this:

    var input = Console.ReadLine();
    var highest = input.Split(',').Select(int.Parse).Max();
    Console.WriteLine(highest);