Issue #41 - Bring method of determining IE version up to date

src/shared/Registry.h

+#ifndef REGISTRY_H
+#define REGISTRY_H
+
 #include <string>

[sergei, 2014/07/28 11:46:27]

Where is the guard?

 #include <Windows.h>
 
 namespace AdblockPlus
 {
   /**
    * An open key in the system registry.
    *
    * This class is not completely general.
    * In particular, it cannot encapsulate the predefined registry keys
    *   such as HKEY_CLASSES_ROOT.
    * These classes are considered "always open",
    *   and the destructor should not close them.
    * Rather than trying to detect these predefined keys,
    *   we simply don't allow them to be constructed.
    * In practice, this is a limitation without much consequence.
    * If this were a library designed for standalone use,
    *   this limitation might not be appropriate, but it's fine here.
    */

[Oleksandr, 2014/07/27 22:13:30]

Extensive comments make the code hard to read, IMHO. This file is a prime
example of that, but in other files I would remove almost all of comments as
well. Even though I am at awe at your willingness to explain everything in
writing :) 
But I won't insist here, unless others agree. @sergei, @fhd?

[sergei, 2014/07/28 11:46:27]

I find this particular comment and the comment for the ctr to be useful, because
they describe the peculiarities of the class behaviour.

[Eric, 2014/07/28 11:48:43]

These comments are formatted for consumption by Doxygen. It would help to see
the Doxygen output to get an idea what it looks like.

-  class Registry_Key

[sergei, 2014/07/28 11:46:27]

remove underscore

+  class RegistryKey
   {
     /**
      * Handle to registry key that is open and not predefined.

[sergei, 2014/07/28 11:46:27]

But, for example, this comment and the comment for the dtr are not necessary.

      */
     HKEY key;
 
   public:
     /**
      * Constructor to open a key as a subkey key of an existing parent.
      * Opens the key with read-only access.
      *
      * The constructor throws if 'key_name' is not found,
      *   to preserve the invariant that the registry key is open.
      * The constructor also throws if 'key_name' is empty,
      *   to preserve the invariant the the registry key is not predefinded.
      *
      * \param parent
      *   An open registry key. This may be one of the predefined keys.
      * \param key_name
      *   Name of the subkey to be opened under the parent.
      */
-    Registry_Key(HKEY parent, std::wstring key_name);

[sergei, 2014/07/28 11:46:27]

- const references
- keyName

+    RegistryKey(HKEY parent, const std::wstring& key_name);
 
     /**
      * Destructor always closes the key.
      */
-    ~Registry_Key();
+    ~RegistryKey();
 
     /**
      * Retrieve a value from a name-value pair within the present key.
      *
      * Throws if the name is not found within the dictionary.
      * Throws if the name is found but is not a string.
      */
-    std::wstring value_wstring(std::wstring name) const;

[sergei, 2014/07/28 11:46:27]

To be consistent with the rest code, it's better to name it `StringValue`

+    std::wstring value_wstring(const std::wstring& name) const;
   };
 }
 
+#endif