6
\$\begingroup\$

Please see the code options below:

Client App - this is the same for both options

LetterCounter lc = new LetterCounter("HeLLo");

Option 1

class LetterCounter
    {
        string _Word;
        List<LetterQuantities> letterQuantities;

        //Other functions will be here.

        public LetterCounter(string Word)
        {
            _Word = Word;
            LetterQuantities lq = new LetterQuantities();
            letterQuantities = lq.GetCapialLetterQuantities(Word);
        }
    }

    class LetterQuantities
    {
        int _Quantity;
        char _Letter;

        public LetterQuantities()
        {
        }

        public LetterQuantities(int Quantity, char Letter)
        {
            _Quantity = Quantity;
            _Letter = Letter;
        }

        public List<LetterQuantities> GetCapialLetterQuantities(string Word)
        {
            List<LetterQuantities> letterQuantitiesList = new List<LetterQuantities>();
            for (char c = 'A'; c <= 'Z'; c++)
            {
                int count = Word.Split(c).Length - 1;
                if (count > 0)
                {
                    LetterQuantities lq = new LetterQuantities(count, c);
                    letterQuantitiesList.Add(lq);
                }
            }
            return letterQuantitiesList;
        }
    }

Option 2

class LetterCounter
    {
        string _Word;
        List<KeyValuePair<char, int>> LetterQuantities = new List<KeyValuePair<char, int>>();

        //Other functions will be here.

        public LetterCounter(string Word)
        {
            _Word = Word;
            GetCapialLetterQuantities();
        }

        public void GetCapialLetterQuantities()
        {
            for (char c = 'A'; c <= 'Z'; c++)
            {
                int count = _Word.Split(c).Length - 1;
                if (count > 0)
                {
                    LetterQuantities.Add(new KeyValuePair<char,int>(c,count));
                }
            }
        }
    }

The concerns I have are:

1) Option 1: LetterQuantities returns a list of itself. Not sure this is ideal. However, it is how a Tree data structure works. 2) Option 2: This is simpler. However, I am not sure whether 'Letter' is a valid key. 3) I am not sure whether the object(s) should be Singletons as they are value objects rather than entity objects.

I believe I should use option 1 from a TDD point of view.

\$\endgroup\$
  • \$\begingroup\$ What about "abc123".Count(char.IsLetter) ? Oh I see. Could you please clarify what your code is suposed to do? Otherwise other people will have the same problem as me/ \$\endgroup\$ – Bruno Costa Jun 1 '17 at 15:28
  • \$\begingroup\$ @ Bruno Costa, I am trying to understand whether to use a class or a dictionary for the LetterQuantities. \$\endgroup\$ – w0051977 Jun 1 '17 at 15:33
7
\$\begingroup\$

The answer is that I wouldn't use either of those approaches. Counting the number of occurrences on a string isn't complex enough that requires a class of it's own. An extension method however would be a reasonable way to do it, as well as using the LINQ to get the desired result:

public static IDictionary<char, int> GetOcurrences(this string value){
    return value.GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}

Or to count only letters

public static IDictionary<char, int> GetOcurrences(this string value){
    return value
        .Where(char.IsLetter)
        .GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}

Or to count only capital letters

public static IDictionary<char, int> GetOcurrences(this string value){
    return value
        .Where(c => c >= 'A' && c <= 'Z')
        .GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}

Or to count only letters provided by a parameter

public static IDictionary<char, int> GetOcurrences(this string value, string filter){
    return value
        .Where(c => filter.Contains(c))
        .GroupBy(c => c)
        .ToDictionary(c => c.Key, c => c.Count());
}
\$\endgroup\$
  • \$\begingroup\$ Is a character suitable for a key? Thanks \$\endgroup\$ – w0051977 Jun 1 '17 at 15:52
  • \$\begingroup\$ @w0051977 Yes it is, as suitable as an integer, or a string \$\endgroup\$ – Bruno Costa Jun 1 '17 at 15:52
  • \$\begingroup\$ thanks. Is a decimal (currency) suitable for a key? \$\endgroup\$ – w0051977 Jun 1 '17 at 15:59
  • \$\begingroup\$ @w0051977 I am not totally sure about that one but I never seen it being used as a key. If you need to use it has a key you might be doing something wrong as well. The problem is that even if the decimal type might implement a good GetHashCode there some cases where you have a key 0.3333 present on the dictionary but try to fetch 0.33333333333333 instead \$\endgroup\$ – Bruno Costa Jun 1 '17 at 16:08
  • \$\begingroup\$ The key would be for currency groupings i.e. 50M,20M,10M,5M,2M,1M,05M,0.2M,0.1M,0.05M,0.02M,0.01M. \$\endgroup\$ – w0051977 Jun 1 '17 at 16:13
1
\$\begingroup\$

I would prefer to use IEnumerable<char> instead of string. It's more generic solution.


I like the @bruno-costa' answer, but instead of Dictionary I would suggest to use just array of int.

    public static int[] GetOccurrences(IEnumerable<char> value) {
        int[] result = new int[char.MaxValue + 1];
        foreach (char c in value) {
            result[c]++;
        }
        return result;
    }

    public static int[] GetCapialLetterOccurrences(IEnumerable<char> value) {
        int[] capialLetterOccurrences = new int['Z' - 'A' + 1];
        int[] letterOccurrences = GetOccurrences(value);
        Array.Copy(letterOccurrences, 'A', capialLetterOccurrences, 0, capialLetterOccurrences.Length);
        return capialLetterOccurrences;
    }

You may say that \$2^{16}\$ bytes is an overkill for such task and I agree

    public static int[] GetCapialLetterOccurrences_upd(IEnumerable<char> value) {
        int[] result = new int['Z' - 'A' + 1];
        foreach (char c in value) {
            if ('A' <=c && c <= 'Z') {
                result[c]++;
            }
        }
        return result;
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ creating an array of 2^16 entries doesn't seem to be a good idea. \$\endgroup\$ – Bruno Costa Jun 2 '17 at 2:32
  • \$\begingroup\$ It perfectly fits to modern L1 cache, so do not expect any performance overhead. You actually can use 2^('Z'-'A'+1) for capitals with prefiltering. \$\endgroup\$ – pgs Jun 2 '17 at 7:46
  • \$\begingroup\$ @BrunoCosta but I agree with you - for finding capital letters occurrences it's an overkill. \$\endgroup\$ – pgs Jun 2 '17 at 8:21
  • 1
    \$\begingroup\$ Did you mean log2(2^('Z'-'A'+1)) = ('Z'-'A'+1) ;) \$\endgroup\$ – Bruno Costa Jun 2 '17 at 8:24
  • \$\begingroup\$ @BrunoCosta OMG)) Of course I meant "It perfectly fits to modern L1 cache, so do not expect any performance overhead. You actually can use 'Z'-'A'+1 for capitals with prefiltering." Sorry! \$\endgroup\$ – pgs Jun 2 '17 at 8:36

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.