How can I efficiently compare two sets of strings in C#?

Question

i have two variables (well basically four => old/current for each one). all of them can be "ABCDXFG" or ":ABCDXFG" (basically level names) and also "hub".

I have an if else statement to return true if Level names do not equal, but I need exceptions for the hub (it's really long even now) plus another 4 levels!!!

if (current.LevelSecond != old.LevelSecond && 
    current.LevelSecond != old.LevelFirst && 
    current.LevelFirst != "HUB" && 
    old.LevelFirst != "HUB" && 
    current.LevelSecond != "HUB" && 
    old.LevelSecond != "HUB")
        return true;

else if (current.LevelFirst == "HUB" || 
         old.LevelFirst == "HUB" || 
         current.LevelSecond == "HUB" || 
         old.LevelSecond == "HUB")
            return false;

this looks and is absolutely disgusting, and since I have zero experience I don't know how to make this as efficient as possible. If I add those 4 levels the length of the statement is gonna quadruple...

Please help! :(

well you dont need the word 'else' - so that shortens it a bit — pm100, Jul 12 '17 at 19:06
first it`s really good idea to store "HUB" as a const string for a start. — Vladimir, Jul 12 '17 at 19:07
If you put your second condition first, you can remove the "HUB" checks from the first one. — Rufus L, Jul 12 '17 at 19:07
Awesome Rufus, you are one hundred percent right! That only makes it a little bit shorter though, I need to add another four values. Let's say = level1, level2, level3, level4 (simplified of course) — NoobishCoder, Jul 12 '17 at 19:11
@Noobish - I hope you don't mind, I changed the title of your question as I think the original wording gave the impression that your problem revolved specifically around language constructs of if-else and try-catch (exceptions). Your goal - an efficient solution to your comparison problem - will almost certainly be accomplished without using if-else at all, but collections of some sort (arrays, sets, lists etc) so by summarising that problem rather than describing your current approach, hopefully people with an insight into such algorithms may be drawn to help. — Deltics, Jul 12 '17 at 19:12
@NoobishCoder is there a third option besides `true` and `false`? Otherwise one of these should just be an `else` — River, Jul 12 '17 at 19:12
@Deltics thank you, how would I go on about doing this with lists/sets? Like I said I'm a complete noob. — NoobishCoder, Jul 12 '17 at 19:17
@NoobishCoder so why not test for any equal to `"HUB"` and otherwise `return true`? — River, Jul 12 '17 at 19:18
It might help if you described the actual reasoning behind your code. — shmosel, Jul 12 '17 at 20:53
@NoobishCoder also look here https://stackoverflow.com/q/22501230/3745896 — River, Jul 17 '17 at 19:42

score 0 · Answer 1 · answered Jul 12 '17 at 19:28

One possibility might be to push part of this detection functionality into your class, which should simplify the code you're currently writing, and potentially reduce other code if it's reused somewhere else:

class Item
{
    public const string Hub = "HUB";
    public string LevelFirst { get; set; }
    public string LevelSecond { get; set; }

    public bool HasCommonLevelWith(Item other)
    {
        if (other == null || ContainsAHub() || other.ContainsAHub())
        {
            return false;
        }

        return
            LevelFirst.Equals(other.LevelFirst, StringComparison.OrdinalIgnoreCase) ||
            LevelSecond.Equals(other.LevelSecond, StringComparison.OrdinalIgnoreCase);
    }

    public bool ContainsAHub()
    {
        return LevelFirst.Equals(Hub, StringComparison.OrdinalIgnoreCase) ||
               LevelSecond.Equals(Hub, StringComparison.OrdinalIgnoreCase);
    }
}

Then the code you wrote earlier would simply look like:

return current.HasCommonLevelWith(old);

Deltics · Answer 2 · 2017-07-12T19:40:15.587

Here is an approach you can use that uses a collection of a simple data structure to hold your Old and Current level values:

class Level
{
    public string Current { get; set; }
    public string Old { get; set; }
}

class Program
{

    static bool CompareLevels(ICollection<Level> levels)
    {
        foreach (var item in levels)
        {
            // Either of any level is HUB => false
            if ((item.Current == "HUB") || (item.Old == "HUB"))
                return false;

            // Any Old level == any Current level => false
            if (levels.Any(level => level.Old == item.Current))
                return false;
        }

        // All levels met the criteria (no exceptions/failures)
        return true;
    }


    // A simple console program to exercise the above function:

    static void Main(string[] args)
    {
        // Should return false if any Old or Current levels are HUB
        var levels = new List<Level>
        {
            new Level { Current = "HUB", Old = "ABCDEF" },
            new Level { Current = "DEFGHI", Old = "HUB" }
        };
        Console.WriteLine(CompareLevels(levels));

        // Should return true if all Old levels != any Current level and none are "HUB"
        levels = new List<Level>
        {
            new Level { Current = "ABC123", Old = "ABCDEF" },
            new Level { Current = "DEFGHI", Old = "DEF456" }
        };
        Console.WriteLine(CompareLevels(levels));

        // Should return false if any Old level == any Current level
        levels = new List<Level>
        {
            new Level { Current = "ABC123", Old = "ABCDEF" },
            new Level { Current = "DEFGHI", Old = "ABC123" }
        };
        Console.WriteLine(CompareLevels(levels));

        Console.ReadLine();
    }
}

The iteration of the collection of levels will terminate at the first failure condition so will not wastefully iterate over the whole collection. The only condition under which the collection is completely enumerated is when there are no failures.

This may not be the most efficient but optimizations here will be vanishingly small for the numbers of levels you indicate are involved. It also requires that you initialise a collection to hold the levels to be compared.

It does however have the advantage of accommodating any number of "levels" and (imho) making the comparison criteria very clear and easily understood.

NOTE: In C# there is no problem using == to compare string values, other than when needing to take into account culture and case variations etc. == for strings will compare the value of the strings involved.

How can I efficiently compare two sets of strings in C#?

2 Answers2