How to add from an array of strings to a list, each word that has its letters in alphabetical order?-CodePudding

string[] words = new string[5] { "abbey","billowy", "chills","abced","abcde" };

it should display only:

abbey billowy chills abcde

I tried this code

List<string> AlphabeticOrder = new List<string>();
foreach (var word in words)
{
    for (int i = 1; i < word.Length; i  )
    {
        if (word[i] < word[i - 1])
        {
            break;
        }
        AlphabeticOrder.Add(word);
        break;
    }
}

CodePudding user response：

One line solution:

var alphabeticOrder = words.Where(w => Enumerable.SequenceEqual(w.OrderBy(x => x), w)).ToList();

CodePudding user response：

This becomes easier if you break it into pieces. What you need is a function that takes a string and tells you if the characters in the string are in alphabetical order.

For example:

public static class CharacterSequence // I didn't think hard about the name
{
    public static bool CharactersAreInAlphabeticalOrder(string input)
    {
        return input.SequenceEqual(input.OrderBy(c => c));
    }
}

Having done that, the next part is just checking a collection of strings and returning only the ones where the characters are in order. If A string is a collection of characters (char). This method takes the sequence of characters and sorts it. Then it compares the original to the sorted. If they are the same, then the original sequence was in order.

var wordsWithCharactersInOrder = 
    words.Where(CharacterSequence.CharactersAreInAlphabeticalOrder);

One reason why it's helpful to break it up like this is that it's easier to understand. It's very easy to read the above code and tell what it does. Also, if you realize that there's something you want to change about the way you check for characters in order, you can change that in the smaller function.

For example, you might realize that the original function is case-sensitive. C comes before d, but D comes before c. In this example it's less noticeable because the function is small, but as logic becomes more complex it's easier to read and think about when we break things into smaller functions. The case-insensitive version would be

public static bool CharactersAreInAlphabeticalOrder(string input)
{
    var lowerCase = input.ToLower();
    return lowerCase.SequenceEqual(lowerCase.OrderBy(c => c));
}

If you want to go a step further then you can compare the characters one at a time instead of sorting the entire string.

public static bool CharactersAreInAlphabeticalOrder(string input)
{
    if (input.Length < 2) return true;
    var lowerCase = input.ToLower();
    var characterIndexes = Enumerable.Range(0, input.Length - 1);
    return characterIndexes.All(characterIndex => 
        lowerCase[characterIndex] <= lowerCase[characterIndex   1]);
}

You can also write unit tests for it. If you know that the smaller function always returns the expected results, then the larger one that checks a collection of strings will return the correct results.

Here's an example of a unit test. It's much easier to test lots of conditions this way and have confidence that the function works than to edit the code and run it over and over. If you realize that there's another case you have to account for, you can just add it.

[DataTestMethod]
[DataRow("A", true)]
[DataRow("AB", true)]
[DataRow("abc", true)]
[DataRow("aBc", true)]
[DataRow("ba", false)]
public void CharactersAreInAlphabeticalOrder_returns_expected_result(string input, bool expected)
{
    var result = CharacterSequence.CharactersAreInAlphabeticalOrder(input);
    Assert.AreEqual(expected, result);
}

There was a small error in my original code. It didn't work if a word had only two letters. Without the test that error could have gone into the application without being noticed until later when it would take longer to find and fix. It's much easier with a test.

CodePudding user response：

Words with letters in alphabetical order are known as abecedarian.

The difficulty in your algorithm is breaking out of a nested loop. There are different strategies to solve this problem:

Use a labeled statement and goto. Goto is frowned upon.
Use of a Boolean guard. This is okay but not very readable.
Place the inner loop into a method. This is the clean and easy to read solution that I decided to present.

Let us create a helper method:

private static bool IsAbecedarianWord(string word)
{
    for (int i = 1; i < word.Length; i  ) {
        if (word[i] < word[i - 1]) {
            return false;
        }
    }
    return true;
}

With its help we can write:

foreach (var word in words) {
    if (IsAbecedarianWord(word)) {
        AlphabeticOrder.Add(word);
    }
}

Clean and simple!

One note to naming conventions in C#. The usual conventions are (in short):

Type names, Method names and Property names are written in PascalCase. Interfaces are additionally prefixed with an upper case I (IPascalCase).
Names of method parameters and local variables are written in camelCase.
Field names (class and struct variables) are written in _camelCase with a leading underscore.

With that in mind, I suggest renaming AlphabeticOrder to abecedarian.

CodePudding user response：

If you want to use your method try adding this:

foreach (var word in words)
{
    for (int i = 1; i < word.Length; i  )
    {
        if (word[i] < word[i - 1])
        {
            break;
        }
        if (i == word.Length - 1)
        {
            AlphabeticOrder.Add(word);
        }
    }
}

Problem in your code is that it checks first 2 letters and if they are in alphabetic order it adds them to list.

CodePudding user response：

The reason this is not working as expected is because the logic on whether to discard a result is flawed. The for loop which iterates the letters within the word is only checking the first letter and then exiting the loop regardless of the result.

I've added comments to your function below to help explain this.

for (int i = 1; i < word.Length; i  )
{
    if (word[i] < word[i - 1]) // check the 2nd letter of the word against the 1st
    {
        break; // if the 2nd letter comes before the 1st in the alphabet, exit
    }
    AlphabeticOrder.Add(word); // add the word to the list
    break; // exit the for loop
}

You should refactor the code such that it checks every letter of the word before adding it to the list of alphabetical words. You can also still end the for loop early if the condition fails.

There's a few ways to solve this, you could track the letters like Adam's answer above. Another possibility is to sort the array of letters and compare it to the original. If the arrays match then it's an alphabetical word for your scenario, if no match then it's not.

E.g.

foreach (var word in words)
{
    var letters = word.ToList();
    var sorted = word.OrderBy(l => l);
    if (letters.SequenceEqual(sorted))
    {
        AlphabeticOrder.Add(word);
    }  
}

Which outputs:

abbey,billowy,chills,abcde

CodePudding user response：

Logic is flawed.
Condition is satisfied in the first 2 letters and immediately added to the list.

List<string> AlphabeticOrder = new List<string>();
bool isOrdered = true; // Added this
foreach (var word in words)
{
    isOrdered = true; // Added this
    for (int i = 1; i < word.Length; i  )
    {
        if (word[i] < word[i - 1])
        {
            isOrdered = false; // Added this
            break;
        }
    }
            
    // Added this
    if(isOrdered)
        AlphabeticOrder.Add(word);
}