1

The first if statement in this program checks a string against a letter I am using the logical or operator (||) to compare the first letter against the string. It's tedious and for the future I am looking for a way to this faster and make it shorter.

string strUser;
string strSubCheck;
Console.WriteLine("Please type in a word:");
strUser = Console.ReadLine();
strSubCheck = strUser.Substring(0, 1);    

if(strSubCheck == "A" || strSubCheck == "a" || strSubCheck == "E" || strSubCheck == "e" || strSubCheck == "I" || strSubCheck == "i" || strSubCheck == "O" || strSubCheck ==  "o" || strSubCheck == "U" || strSubCheck == "u")
    {
      Console.WriteLine("\nThe first letter is a vowel");
    }
else Console.WriteLine("\nThe first letter is a consonant");
George Stocker
  • 57,289
  • 29
  • 176
  • 237
Adib
  • 29
  • 4
  • If you want someone to review your code; go to codereview.stackexchange.com; if you want your question re-opened here, we need a title and problem statement that other people would search for if they had your problem. No one would search for "is there any simpler and shorter way to do this"; tell us the problem you have; with words that you'd you'd use to search for to solve this problem. – George Stocker Mar 23 '16 at 15:26
  • ^ what he said. also on Code Review because *every single question* is literally asking for a *shorter and neater way to do this*, they'll want a title that tells potential reviewers *what your code is doing*. – Mathieu Guindon Mar 23 '16 at 15:28
  • 3
    @GeorgeStocker I don't think that closing this question outright was a good course of action. The question is specific enough to have a good answer, except for its title, which could definitely be improved (which I did). – Sergey Kalinichenko Mar 23 '16 at 15:28
  • Ive changed the title now, sorry guy this is my first time on the site – Adib Mar 23 '16 at 15:34
  • MatthewWatson has a good answer for a specific case. If you're looking for more of a general case, I would suggest a helper method: http://ideone.com/KV08WD – David Mar 23 '16 at 15:37
  • @dasblinkenlight We close questions so that the OP (or others) can make them better (and good enough to be open). If we didn't do that, we'd have the site littered with terrible questions with no google-juice for future visitors; and no incentive for users to fix their questions. – George Stocker Mar 23 '16 at 15:37
  • I agree with George, my previous title was pretty bad – Adib Mar 23 '16 at 15:41

2 Answers2

9

This will give you the same answer as your code, and is easier to extend:

bool beginsWithVowel = "aAEeIiOoUu".Contains(strUser[0]);

Or avoiding having to have upper and lower case (not guaranteed to work for all cultures):

bool beginsWithVowel = "AEIOU".Contains(char.ToUpper(strUser[0]));

(And check that strUser isn't null before doing this, of course...)

If you wanted to make this work for all cultures without specifying the upper and lower case vowels explicitly, you could use string.IndexOf() as mentioned by Nyerguds below. However, if you do that, it starts to get significantly more complicated and at that point I think I'd rather just use the version that specifies the upper and lower characters explicitly:

bool beginsWithVowel = "AEIOU".IndexOf(new string(strUser[0], 1), StringComparison.CurrentCultureIgnoreCase) >= 0;

(See What is the correct way to compare char ignoring case? for more details about case insensitive comparison of chars.)

Community
  • 1
  • 1
Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
  • 1
    @Downvoter: Care to explain what is wrong with this answer? – Matthew Watson Mar 23 '16 at 15:28
  • 4
    I didn't downvote but there's a possible Exception if the string is empty. On top of that it would be more efficent to maintain only one sort of chars and convert the case programatically. – Stefan Wanitzek Mar 23 '16 at 15:29
  • 2
    String.IndexOf has an IgnoreCase option, so using that is more compact since you only need them in one case. You'll need to check on >= 0 then though, of course. – Nyerguds Mar 23 '16 at 15:31
  • 3
    Oh, and to those ignorant of languages outside English... calling `ToUpper()` does not magically make it a case insensitive check. – Nyerguds Mar 23 '16 at 15:34
  • @Nyerguds Yes, but that version of `IndexOf` expects a string as the target to be searched for. – Matthew Watson Mar 23 '16 at 15:34
  • @matthewatson thanks for the answer, works great now – Adib Mar 23 '16 at 15:46
1

Be aware that shorter is not always better. As long as it is clearly readable what is happening in the code should be the priority.

string strUser;
string strSubCheck;
Console.WriteLine("Please type in a word:");
strUser = Console.ReadLine();
strSubCheck = strUser.Substring(0, 1);
var vowelCheck = new[] { "a", "e", "i", "o", "u" };

if (vowelCheck.Contains(strSubCheck.ToLower()))
{
    Console.WriteLine("\nThe first letter is a vowel");
}
else Console.WriteLine("\nThe first letter is a consonant");
James Dev
  • 2,979
  • 1
  • 11
  • 16