Clean Code attempt of HOLES problem on codechef.com The problem asks you to take read an integer (number of words to read) and then process words entered counting the number of "HOLES" exist.  Letters with HOLES are A B D O P Q and R.  Of those B has 2 holes A D O P Q R have 1 and any other letter not listed counts as 0 even lowercase equivalents.  

The scope is fairly limited so try to just assess the code based on the problem listed.

Full problem description: here

#include &lt;iostream&gt;
#include &lt;map&gt;
#include &lt;string&gt;

class LetterEvaluator {
public:
    LetterEvaluator()
    :   letterValues(createMap())
    {}

    int getValueOf(const char letter) const {       
        if(retrieveValueOfLetter(letter) == letterValues.end())
            return 0;
        else{
            return retrieveValueOfLetter(letter)-&gt;second;
        }
    }

private:
    const std::map&lt;char,int&gt; letterValues;

    std::map&lt;char,int&gt; createMap() const {
        std::map&lt;char,int&gt; initializedMap;
        initializedMap['A'] = 1;
        initializedMap['B'] = 2;
        initializedMap['D'] = 1;
        initializedMap['O'] = 1;
        initializedMap['P'] = 1;
        initializedMap['Q'] = 1;
        initializedMap['R'] = 1;
        return initializedMap;
    }

    std::map&lt;char,int&gt;::const_iterator retrieveValueOfLetter(const char letter) const {
        return letterValues.find(letter);
    }
};

class HolesInCapitalizedWordFinder {
public:
    HolesInCapitalizedWordFinder()
        :   numberOfHoles(0)
        ,   letterEvaluator()
    {}

    void processWord() {        
        evaluate(readWord());
        printNumberOfHolesInWord();
    }

private:
    LetterEvaluator letterEvaluator;
    int             numberOfHoles;

    std::string readWord() const {
        std::string wordToEvaluate;
        std::cin &gt;&gt; wordToEvaluate;
        return wordToEvaluate;
    }   

    void evaluate(const std::string&amp; wordToEvaluate ) {
        numberOfHoles = 0;
        for(auto&amp; character : wordToEvaluate) {
            if(isNotAnUpperCaseLetter(character)){
                numberOfHoles = -1;
                break;
            }               
            numberOfHoles += letterEvaluator.getValueOf(character);
        }       
    }

    bool isNotAnUpperCaseLetter(const char character) const{
        return character &lt; 65 || character &gt; 90;
    }

    void printNumberOfHolesInWord() const {
        if(numberOfHoles == -1) {
            std::cout &lt;&lt; "Not all characters in that word are uppercase letters!\n";
        }
        else {
            std::cout &lt;&lt; numberOfHoles &lt;&lt; '\n';
        }
    }
};


int main() {
    std::ios::sync_with_stdio(false);

    int timesToRun = 0;
    std::cin &gt;&gt; timesToRun;

    HolesInCapitalizedWordFinder holesInCaptilizedWordFinder;

    for(int numberOfTimesRan = 1; numberOfTimesRan &lt;= timesToRun; ++numberOfTimesRan) {
        holesInCaptilizedWordFinder.processWord();
    }
    return 0;
}
LetterEvaluator


Since this is a C++11 solution, you no longer need something like createMap():


std::map&lt;char,int&gt; createMap() const {
        std::map&lt;char,int&gt; initializedMap;
        initializedMap['A'] = 1;
        initializedMap['B'] = 2;
        initializedMap['D'] = 1;
        initializedMap['O'] = 1;
        initializedMap['P'] = 1;
        initializedMap['Q'] = 1;
        initializedMap['R'] = 1;
        return initializedMap;
    }



Use C++11's list initialization to have it initialized in place:

const std::map&lt;char, int&gt; letterMap {
    {'A', 1},
    {'B', 2},
    {'D', 1},
    {'O', 1},
    {'P', 1},
    {'Q', 1},
    {'R', 1}
};

If you have C++14, you can avoid providing an explicit return type:


std::map&lt;char,int&gt;::const_iterator retrieveValueOfLetter(const char letter) const {
        return letterValues.find(letter);
    }



You can use automatic return type deduction via auto:

auto retrieveValueOfLetter(const char letter) const {
    return letterValues.find(letter);
}


This does also shorten the line, which could help add more readability.
You have two redundant calls to retrieveValueOfLetter():


int getValueOf(const char letter) const {       
        if(retrieveValueOfLetter(letter) == letterValues.end())
            return 0;
        else{
            return retrieveValueOfLetter(letter)-&gt;second;
        }
    }



Just retrieve the value from one call and then return the appropriate value:

int getValueOf(const char letter) const {
    const auto retrievedLetter = retrieveValueOfLetter(letter);
    return (retrievedLetter == letterValues.end()) ? 0 : retrievedLetter-&gt;second;
}


I've used a ternary here to shorten the expression to just one line.  If it looks less-readable, or if you may need to expand on it later, you may still use if/else instead.


HolesInCapitalizedWordFinder


It looks like readWord() should be a free function (non-member function).  However, if you're trying to read in a word, then you can just overload operator&gt;&gt; for this class.
Do you really need isNotAnUpperCaseLetter()?  There's already std::isupper() provided in the standard library, which should be safer to use.  You may encounter some issues with your own, depending on how it's implemented.