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

src/shared/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

+  {
+    /**
+     * 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

+
+    /**
+     * Destructor always closes the key.
+     */
+    ~Registry_Key();
+
+    /**
+     * 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`

+  };
+}
+