Cfxtrjtrk

An Isogram (also known as a "nonpattern word") is a word or phrase without a repeating letter, however spaces and hyphens are allowed to appear multiple times.

IsIsogram() method takes a string and returns boolean value.

1. INPUT: anik OUTPUT: true


2. INPUT: A nika OUTPUT: false

Approach 01: Using Dictionary

static bool IsIsogram(string str)

    {

        //create a dictionary with 26 keys and assign false as default value

        var storeOccurance = new Dictionary<int, bool>();

        for (int i = 0; i <= 25; i++)

        {

            storeOccurance[i] = false;

        }



        str = Regex.Replace(str, "[ -]", ""); //ignore whitespace and dash



        //iterate over each character of the string

        foreach (var ch in str.ToLower())

        {

            //(ch - 'a') is used to calculate the key. Check if the character key has false as it's value,

            //meaning they were never stored

            if (!storeOccurance[ch - 'a'])

            {

                storeOccurance[ch - 'a'] = true; //assign true when they are stored in the dictionary

            }

            else //otherwise, if a character key has true as it's value then it means that it exists in the dictionary 

            {                   

                return false; //return false and exits the iteration..Multiple occurance in dictionary

                               //means the string has repeating character so it's not Isogram

            }

        }

        return true;

    }

Approach 02: Using HashSet

static bool IsIsogram(string str)

    {

        //TRICK: HashSet<T>.Add(T) returns false if an item already exists in HashSet<T>,

        //otherwise returns true and add the character to the datastructure



        var charHash = new HashSet<char>();



        foreach (var ch in str.ToLower())

        {   

            //check if the charHash.Add() returns false, if it does then terminate

            //as it means the character already exists

            if (!charHash.Add(ch) && char.IsLetter(ch))

                return false;               

        }

        return true; //otherwise return true

    }

Approach 03: Using LINQ

static bool IsIsogram(string str)

    {                                    //Suppose "anik a" is a given string

        return str.Where(char.IsLetter) //filter only the letters ||here- only a, n, i, k, a will be taken 

                  .GroupBy(char.ToLower) //create group by characters(and transform them to lowercase) || here, there will be a group for each character a, n, i, k

                  .All(g => g.Count() == 1); //for every group, count the number of it's element and check if it's 1 

                                            //|| here, 'a' group has 2 elements so it return false though other groups has only one element in their group

    }

Given those above approaches,

1. Which approach would you recommend regarding readability, clean code and performance?


2. Is there any scope for improvement in the existing approaches?

Choice of Map Key

Ultimately, as you are querying your map based upon the character, you should really make your key as the character. This makes your regular subtraction of 'a' unnecessary.

    var storeOccurance = new Dictionary<char, bool>();

    for (char c = 'a'; c <= 'z'; c++)

    {

        storeOccurance[c] = false;

    }

Inconsistent behaviour for non-letter characters

Given a string such as anik00, the first approach will produce a false response, as the duplicate 0s are detected like any other letter. The other two approaches will produce true, as 0s are ignored.

Comments

I don't think these comments are necessary, as the code is clear enough on its own. If you feel the need to add comments to explain what something is doing, you should extract methods instead to make these intentions clear.

Which approach would I choose?

The litmus test is how easy it is to understand. I've not seen any serious LINQ for a number of years, and despite that I can understand your LINQ query without difficulty. If you can create understandable code in fewer lines, it's a good thing.

I like the second approach since it allows you to exit as soon as a false condition is reached and is simpler than the first. I would offer one minor optimization, keep the check for isletter separate and only check for duplicate if isletter is true:

static bool IsIsogram(string str)

{

    //TRICK: HashSet<T>.Add(T) returns false if an item already exists in HashSet<T>,

    //otherwise returns true and add the character to the datastructure



    var charHash = new HashSet<char>();



    foreach (var ch in str.ToLower())

    {   

        //check if the charHash.Add() returns false, if it does then terminate

        //as it means the character already exists

        if(char.IsLetter(ch))

        {

            if (!charHash.Add(ch))

                return false; 

        }



    }

    return true; //otherwise return true

}

I suggest using an array of boolean, so instead of

var storeOccurance = new Dictionary<int, bool>();

use simply

var storeOccurance = new bool[26];

Although if you want to allow a large character set instead of just a-z, say the whole of unicode, the HashSet approach may be appropriate, although in that case you would have to consider surrogates ( the situation where a character is represented by two chars ).

I think i would go for LINQ approach. For this simple operation, it is short and easy to understand. But just for fun i try different approach :

public static bool IsIsogram( string input){

    if (string.IsNullOrEmpty(input))

    {

        throw new ArgumentNullException(nameof(input));

        //or we can return true , no character,  no repeated character :) 

    }

    var preprocessedInput = Regex.Replace(input.ToLower(), "[ -]", "");

    if (input.Length==1)

    {

        return true;

    }

    var firstHalf = preprocessedInput.Substring(0,preprocessedInput.Length/2);

    var secondHalf = preprocessedInput.Remove(0,preprocessedInput.Length/2);



    if (firstHalf.Intersect(secondHalf).Any())

    {

        return false;



    return IsIsogram(firstHalf) && IsIsogram(secondHalf);

}

The input string is divided into two string and checked for any intersected characters. If there is not any intersected character then, each string divided and called the method recursively. As i mentioned, this was just for fun.