trjhtr

I am given an integer "n" as an input and must convert it to a binary representation. Then I have to find the maximum number if consecutive 1's in it.

Can you please evaluate the implementation? Any advice on how to achieve better performance?

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;

class Solution 

 static void Main(String args) 
 
 int n = Convert.ToInt32(Console.ReadLine());
 string bin = "";
 int conmax = 0;

 while (n > 0)
 
 if (n % 2 == 0)
 bin += "0";
 else
 bin += "1";

 n = n / 2;
 

 string result = bin.Split('0');

 for (int i = 0; i < result.Length; i++)
 
 if (result[i].Length > conmax)
 conmax = result[i].Length;
 

 Console.WriteLine(conmax);
 

While this implementation works, unless you are purposely reinventing the wheel it is best to take advantage of built in functions.

1. Converting the input integer to binary can be done with a simple string bin = Convert.ToString(n, 2);. As rolfl mentioned in the comments, your implementation is technically incorrect (it still works since you only care about sequential ones) - using built in functions helps avoid this.




2. Instead of writing your own Max implementation, you can use LINQ's Max.




3. Try to choose more descriptive variable names - presumably conmax stands for something like consecutiveMax, but I had to read the rest of your code to make that guess - it wasn't immediately evident. I would probably name the variable maxConsecutiveOnes.




4. A good next step for this program is to handle invalid user input - what should happen if I enter a when you ask for a number? Right now, the program will crash. This probably isn't desired behavior. You could achieve this by using Int32.TryParse and just accept a zero if you receive invalid input or by catching the exception.




5. Don't worry about performance here. With your data types you can have at most 32 consecutive ones. The most optimization that could be done is switching to a bit shifting approach instead of converting the integer to a string. Since this would seriously hurt readability I wouldn't recommend it.




6. You should remove unnecessary using statements to avoid importing code you don't need to. Visual Studio will give you a bunch by default. In this case, you only need using System; and using System.Linq;.

Here's an alternative solution, using the Int32.TryParse suggestion to avoid crashes on invalid input. You might want to check the return value and warn the user to provide a better user experience.

using System;
using System.Linq;

class Solution 

 static void Main(String args) 
 
 int input;
 Int32.TryParse(Console.ReadLine(), out input);

 int maxConsecutiveOnes = Convert.ToString(input, 2)
 .Split('0')
 .Max(s => s.Length);

 Console.WriteLine(maxConsecutiveOnes);