3
\$\begingroup\$

My code takes an input, uses the input to take a number of letters from the start of the alphabet and put them at the end. I intend to use this to create a Caesar style cipher.

Is there a way to do this more elegantly?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace ConsoleApplication
{
    class Program
    {
        static void Main()
        {
            GettingInput gi = new GettingInput();
            var input = gi.GetInput();
            Console.WriteLine("Your input:\n{0}", input);
            List<char> alphabet = new List<char>();


            for (char c = 'a'; c <= 'z'; c++)
            {
                alphabet.Add(c);
            }
            for (int i = 0; i < input; i++)
            {
                alphabet.Add(alphabet[i]);
            }

            for (int i = input ; i > 0; i--)
            {
                alphabet.Remove(alphabet[i-1]);
            }

            for (int i = 0; i < 26; i++) 
            {
                Console.WriteLine(alphabet[i]);
            }

            Console.ReadLine();
        }
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace ConsoleApplication
{
    public class GettingInput
    {
        public int GetInput()
        {
            Console.WriteLine("Please input an int:");
            while (true)
            {
                try
                {
                    int input = Convert.ToInt32(Console.ReadLine());
                    return input;
                }
                catch (Exception ex)
                {
                    Console.WriteLine(ex);
                    Console.WriteLine("Please input a valid int:");
                }

            }

        }

    }
}

All feedback welcome!

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$
  • Instead of create the whole list and modify it subsequently, you could calculate the value directly.

The code could like that:

static void Main()
{
    GettingInput gi = new GettingInput();
    var input = gi.GetInput();
    Console.WriteLine("Your input:\n{0}", input);

    var first = (int)'a';
    var last = (int)'z';
    var length = last - first;

    var query = Enumerable
        .Range(0, length)
        .Select(i => (char)(first + ((input + i) % length)));

    foreach (var number in query)
        Console.WriteLine(number);

    Console.ReadLine();
}
  • You could use int.TryParse(Console.ReadLine(), out input) instead of catching the exception
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Good idea, but I prefer to avoid mutation in LINQ statements and to minimize mutation in general, so there is a smaller opportunity for bugs. You can write it without mutation like this: Enumerable.Range(0, length).Select(i => (char)(first + ((input + i) % length))) \$\endgroup\$ Commented Jul 15, 2016 at 15:05
  • \$\begingroup\$ @RiskyMartin: Good point, thanks. I modified the answer as suggested. \$\endgroup\$
    – JanDotNet
    Commented Jul 17, 2016 at 9:35
2
\$\begingroup\$

I thought I should make your life easier with Linq and also introduce you to advanced way for implementing your program.

  1. This line of code

         List<char> alphabet = new List<char>();    
            for (char c = 'a'; c <= 'z'; c++)
            {
                alphabet.Add(c);
            }
    

    can be written as

    List<char> alphabet= Enumerable.Range('a', 'z' - 'a' + 1).Select(value=> (Char)value).ToList();

First, It creates a range of a-z which are represented in numbers, converts it to character using the explicit cast and converts it to a list .

  1. This line of code

       for(int i = 0; i < input; i++)
            {
                alphabet.Add(alphabet[i]);
            }
    

    could be written as alphabet.AddRange(alphabet.GetRange(0, input));

  2. This Line of code

    for (int i = input ; i > 0; i--)
            {
                alphabet.Remove(alphabet[i-1]);
            }
    

    could be written as alphabet.RemoveRange(0,input);

  3. Please don't do this

    'while (true) {..... }`
    It is regarded as a bad programming practice While true a bad programming practice

  4. Rather than using try and catch use (this was from Parsing an exception

    int i; if(!int.TryParse(i,out value)) { // You caught it without throwing an exception. }

  5. Note rather than catching all exceptions - ArgumentNullException,FormatException,OverflowException are the exceptions that could occur in your scenario.

\$\endgroup\$

Your Answer

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

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