2
\$\begingroup\$

Is there any other way to shorten this code without sacrificing readability? I am fairly new to C#, let alone programming, and I wanted to know if this is as short as this code can possibly become. Feedback would be greatly welcomed.

Side note: was Questions.Questask used properly here, or could there have been a better way?

 class Program
 {
    static void Main(string[] args)
    {
        string name = "";
        while (name.ToLower() != "none")
        {
            Console.WriteLine("Are you Kydd,Leo,Jay,Sha, or Zigg");
            name = Console.ReadLine();

            if (name.ToLower() == "kydd")
                Question.Questask("Input question here", "Kquestion","Link");
            else if (name.ToLower() == "leo")
                Question.Questask("Input question here", "Lquestion", "Link");
            else if (name.ToLower() == "jay")
                Question.Questask("Input question here", "Jquestion", "Link");
            else if (name.ToLower() == "sha")
                Question.Questask("Input question here", "Squestion", "Link");
            if (name.ToLower() == "zigg")
                Question.Questask("Input question here", "Zquestion", "Link");
       }
    }

    class Question
    {
        public static void Questask(string question, string questAnswer, string url)
        {
            Console.WriteLine(question);
            questAnswer = Console.ReadLine();
            if (questAnswer.ToLower() == "yes")
            {
                Console.WriteLine("You are right!");
                Console.ReadKey();
            }
            else
            {
                System.Diagnostics.Process.Start(url);
            }
        }
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Stick the result of ToLower() into a new variable. You'll only have to call ToLower() once that way. \$\endgroup\$ – Robert Harvey Oct 26 '13 at 2:48
  • \$\begingroup\$ For any non-trivial application, the questions and names would be stored in some sort of data repository like an XML file or database, not emblazoned into the actual code itself. \$\endgroup\$ – Robert Harvey Oct 26 '13 at 2:49
  • \$\begingroup\$ You could place "Input Question Here" in a string instead of repeating it many times. Also, do you really want a Console.ReadKey() at the class code? \$\endgroup\$ – NoChance Oct 26 '13 at 2:55
1
\$\begingroup\$

I rewrote this for you, making it easier to understand and making it a little more efficient (although, the size is just a tad bigger then your original). Hopefully it makes a little more sense and you can ask any questions if you need to:

class Program
{
    static void Main(string[] args)
    {
        var names = new List<NameClass>
        {
            new NameClass {Name = "Kydd", Answer = "Kquestion"},
            new NameClass {Name = "Leo", Answer = "Lquestion"},
            new NameClass {Name = "Jay", Answer = "Jquestion"},
            new NameClass {Name = "Sha", Answer = "Squestion"},
            new NameClass {Name = "Zigg", Answer = "Zquestion"}
        };

        string name = "";
        while (name.ToLower() != "none")
        {
            Console.WriteLine("Are you Kydd,Leo,Jay,Sha, or Zigg");
            name = Console.ReadLine().ToLower();

            var a = names.FirstOrDefault(z => z.Name.ToLower() == name);
            if (a != null)
            {
                Questask(a.Question, a.Answer, a.Url);
            }
        }
    }

    private static void Questask(string question, string questAnswer, string url)
    {
        Console.WriteLine(question);
        questAnswer = Console.ReadLine();
        if (questAnswer.ToLower() == "yes")
        {
            Console.WriteLine("You are right!");
            Console.ReadKey();
        }
        else
        {
            System.Diagnostics.Process.Start(url);
        }
    }

    public class NameClass
    {
        public string Name { get; set; }
        public string Question { get; set; }
        public string Answer { get; set; }
        public string Url { get; set; }

        public NameClass()
        {
            Question = "Input question here";
            Url = "Link";
        }
    }
}

The beauty of this code is that you don't need a bunch of if statements. If you need to add more, simply add one more line of code in the names definition.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ It would be helpful if you explained what exactly did you change. \$\endgroup\$ – svick Oct 26 '13 at 11:07
  • \$\begingroup\$ Thank you for this Beauty of a Code It will help me better understand the use of classes which I still struggle with. Just one Question What exactly does names.FirstOrDefault(z => z.Name.ToLower() == name) line of do kinda stumped on it \$\endgroup\$ – Neil HeRnandez Oct 26 '13 at 15:31
  • \$\begingroup\$ It's a LINQ expression that returns the first NameClass in the list names that has a Name.ToLower that equals name or a default value (null). \$\endgroup\$ – jmoreno Oct 26 '13 at 23:33
1
\$\begingroup\$

When I need a simple strategy pattern I'll use a Dictionary. This can drastically reduce code by cutting repetition.

Note the use of StringComparer.InvariantCultureIgnoreCase for comparer to do away with the String.ToLower calls.

private static readonly IDictionary<string, Tuple<string, string>> NamesDictionary =
    new Dictionary<string, Tuple<string, string>>(StringComparer.InvariantCultureIgnoreCase)
    {
        { "kydd", Tuple.Create("question 1", "link1") },
        { "zydd", Tuple.Create("question 2", "link2") },
        ...
    };

private static void DoSomething(string name)
{
    Tuple<string, string> questionAndLinkTuple;

    if (!NamesDictionary.TryGetValue(name, out questionAndLinkTuple))
        return;

    var question = questionAndLinkTuple.Item1;
    var link = questionAndLinkTuple.Item2;

    Question.AskQuestion("Input answer here", question, link);
}
|improve this answer|||||
\$\endgroup\$
  • 1
    \$\begingroup\$ +1 for being the only answer that renames Questask to AskQuestion :) Actually since the class is Question and the method is static, a perhaps even better name would be simply Ask()... \$\endgroup\$ – Mathieu Guindon Oct 26 '13 at 23:09
  • 1
    \$\begingroup\$ @retailcoder I didn't bother with it in my answer, but a more complex example I could understand the need for a Question class. OP's example would have been just fine without Question and instead have a static Program.AskQuestion. \$\endgroup\$ – ta.speot.is Oct 26 '13 at 23:32
1
\$\begingroup\$

I like icemanind's answer, but to give an alternate point of view -- sometimes you just need to get it done.

string[] names = {"kydd","leo","jay","sha","zigg");
if (Array.IndexOf(names, name.ToLower())>-1){
   Question.Questask("Input question here", 
       string.Format("{0}question",name[0].ToUpper()),"Link");
}
|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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