Use template function to avoid code duplication in wxRegConfig

Don't repeat exactly the same code for 3 different types, just use a
template function instead.

Change the existing TryGetValue() helpers to take all output parameters
consistently by pointer, rather than taking only long by pointer and the
other by reference. This allows to isolate the inconsistency of the type
of wxRegKey::QueryValue() parameter in a single place here, hiding it
from the rest of wxRegConfig code.

Add an extra unit test for writing/reading longs to/from wxRegConfig.

include/wx/msw/regconf.h

@@ -88,6 +88,12 @@ protected:
       return self->m_keyLocal;
   }
 
+  // Type-independent implementation of Do{Read,Write}Foo().
+  template <typename T>
+  bool DoReadValue(const wxString& key, T* pValue) const;
+  template <typename T>
+  bool DoWriteValue(const wxString& key, const T& value);
+
   // implement read/write methods
   virtual bool DoReadString(const wxString& key, wxString *pStr) const wxOVERRIDE;
   virtual bool DoReadLong(const wxString& key, long *plResult) const wxOVERRIDE;

src/msw/regconf.cpp

@@ -39,9 +39,9 @@
 // ----------------------------------------------------------------------------
 
 // get the value if the key is opened and it exists
-bool TryGetValue(const wxRegKey& key, const wxString& str, wxString& strVal)
+bool TryGetValue(const wxRegKey& key, const wxString& str, wxString* strVal)
 {
-  return key.IsOpened() && key.HasValue(str) && key.QueryValue(str, strVal);
+  return key.IsOpened() && key.HasValue(str) && key.QueryValue(str, *strVal);
 }
 
 bool TryGetValue(const wxRegKey& key, const wxString& str, long *plVal)
@@ -49,9 +49,9 @@ bool TryGetValue(const wxRegKey& key, const wxString& str, long *plVal)
   return key.IsOpened() && key.HasValue(str) && key.QueryValue(str, plVal);
 }
 
-bool TryGetValue(const wxRegKey& key, const wxString& str, wxMemoryBuffer &plVal)
+bool TryGetValue(const wxRegKey& key, const wxString& str, wxMemoryBuffer* pBuf)
 {
-  return key.IsOpened() && key.HasValue(str) && key.QueryValue(str, plVal);
+  return key.IsOpened() && key.HasValue(str) && key.QueryValue(str, *pBuf);
 }
 
 // ============================================================================
@@ -550,9 +550,10 @@ wxConfigBase::EntryType wxRegConfig::GetEntryType(const wxString& key) const
 // reading/writing
 // ----------------------------------------------------------------------------
 
-bool wxRegConfig::DoReadString(const wxString& key, wxString *pStr) const
+template <typename T>
+bool wxRegConfig::DoReadValue(const wxString& key, T* pValue) const
 {
-    wxCHECK_MSG( pStr, false, wxT("wxRegConfig::Read(): NULL param") );
+    wxCHECK_MSG( pValue, false, wxT("wxRegConfig::Read(): NULL param") );
 
   wxConfigPathChanger path(this, key);
 
@@ -561,7 +562,7 @@ bool wxRegConfig::DoReadString(const wxString& key, wxString *pStr) const
   // if immutable key exists in global key we must check that it's not
   // overridden by the local key with the same name
   if ( IsImmutable(path.Name()) ) {
-    if ( TryGetValue(m_keyGlobal, path.Name(), *pStr) ) {
+    if ( TryGetValue(m_keyGlobal, path.Name(), pValue) ) {
       if ( m_keyLocal.Exists() && LocalKey().HasValue(path.Name()) ) {
         wxLogWarning(wxT("User value for immutable key '%s' ignored."),
                    path.Name().c_str());
@@ -576,88 +577,33 @@ bool wxRegConfig::DoReadString(const wxString& key, wxString *pStr) const
   }
 
   // first try local key
-  if ( (m_keyLocal.Exists() && TryGetValue(LocalKey(), path.Name(), *pStr)) ||
+  if ( (m_keyLocal.Exists() && TryGetValue(LocalKey(), path.Name(), pValue)) ||
-       (bQueryGlobal && TryGetValue(m_keyGlobal, path.Name(), *pStr)) ) {
+       (bQueryGlobal && TryGetValue(m_keyGlobal, path.Name(), pValue)) ) {
     return true;
   }
 
   return false;
 }
 
-// this exactly reproduces the string version above except for ExpandEnvVars(),
+bool wxRegConfig::DoReadString(const wxString& key, wxString *pStr) const
-// we really should avoid this code duplication somehow...
+{
+  return DoReadValue(key, pStr);
+}
 
 bool wxRegConfig::DoReadLong(const wxString& key, long *plResult) const
 {
-    wxCHECK_MSG( plResult, false, wxT("wxRegConfig::Read(): NULL param") );
+  return DoReadValue(key, plResult);
-
-  wxConfigPathChanger path(this, key);
-
-  bool bQueryGlobal = true;
-
-  // if immutable key exists in global key we must check that it's not
-  // overridden by the local key with the same name
-  if ( IsImmutable(path.Name()) ) {
-    if ( TryGetValue(m_keyGlobal, path.Name(), plResult) ) {
-      if ( m_keyLocal.Exists() && LocalKey().HasValue(path.Name()) ) {
-        wxLogWarning(wxT("User value for immutable key '%s' ignored."),
-                     path.Name().c_str());
-      }
-
-      return true;
-    }
-    else {
-      // don't waste time - it's not there anyhow
-      bQueryGlobal = false;
-    }
-  }
-
-  // first try local key
-  if ( (m_keyLocal.Exists() && TryGetValue(LocalKey(), path.Name(), plResult)) ||
-       (bQueryGlobal && TryGetValue(m_keyGlobal, path.Name(), plResult)) ) {
-    return true;
-  }
-
-  return false;
 }
 
 #if wxUSE_BASE64
 bool wxRegConfig::DoReadBinary(const wxString& key, wxMemoryBuffer *buf) const
 {
-    wxCHECK_MSG( buf, false, wxT("wxRegConfig::Read(): NULL param") );
+  return DoReadValue(key, buf);
-
-  wxConfigPathChanger path(this, key);
-
-  bool bQueryGlobal = true;
-
-  // if immutable key exists in global key we must check that it's not
-  // overridden by the local key with the same name
-  if ( IsImmutable(path.Name()) ) {
-    if ( TryGetValue(m_keyGlobal, path.Name(), *buf) ) {
-      if ( m_keyLocal.Exists() && LocalKey().HasValue(path.Name()) ) {
-        wxLogWarning(wxT("User value for immutable key '%s' ignored."),
-                   path.Name().c_str());
-      }
-
-      return true;
-    }
-    else {
-      // don't waste time - it's not there anyhow
-      bQueryGlobal = false;
-    }
-  }
-
-  // first try local key
-  if ( (m_keyLocal.Exists() && TryGetValue(LocalKey(), path.Name(), *buf)) ||
-       (bQueryGlobal && TryGetValue(m_keyGlobal, path.Name(), *buf)) ) {
-    return true;
-  }
-
-  return false;
 }
 #endif // wxUSE_BASE64
 
-bool wxRegConfig::DoWriteString(const wxString& key, const wxString& szValue)
+template <typename T>
+bool wxRegConfig::DoWriteValue(const wxString& key, const T& value)
 {
   wxConfigPathChanger path(this, key);
 
@@ -666,32 +612,23 @@ bool wxRegConfig::DoWriteString(const wxString& key, const wxString& szValue)
     return false;
   }
 
-  return LocalKey().SetValue(path.Name(), szValue);
+  return LocalKey().SetValue(path.Name(), value);
+}
+
+bool wxRegConfig::DoWriteString(const wxString& key, const wxString& szValue)
+{
+  return DoWriteValue(key, szValue);
 }
 
 bool wxRegConfig::DoWriteLong(const wxString& key, long lValue)
 {
-  wxConfigPathChanger path(this, key);
+  return DoWriteValue(key, lValue);
-
-  if ( IsImmutable(path.Name()) ) {
-    wxLogError(wxT("Can't change immutable entry '%s'."), path.Name().c_str());
-    return false;
-  }
-
-  return LocalKey().SetValue(path.Name(), lValue);
 }
 
 #if wxUSE_BASE64
 bool wxRegConfig::DoWriteBinary(const wxString& key, const wxMemoryBuffer& buf)
 {
-  wxConfigPathChanger path(this, key);
+  return DoWriteValue(key, buf);
-
-  if ( IsImmutable(path.Name()) ) {
-    wxLogError(wxT("Can't change immutable entry '%s'."), path.Name().c_str());
-    return false;
-  }
-
-  return LocalKey().SetValue(path.Name(), buf);
 }
 #endif // wxUSE_BASE64
 

tests/config/regconf.cpp

@@ -42,6 +42,8 @@ TEST_CASE("wxRegConfig::ReadWrite", "[regconfig][config][registry]")
     config->SetPath("/group2");
     CHECK( config->Write("entry1", "bar") );
 
+    CHECK( config->Write("int32", 1234567) );
+
     // test reading
     wxString str;
     long dummy;
@@ -54,6 +56,8 @@ TEST_CASE("wxRegConfig::ReadWrite", "[regconfig][config][registry]")
     CHECK( str == "group2" );
     CHECK( config->Read("group2/entry1", "INVALID DEFAULT") == "bar" );
 
+    CHECK( config->ReadLong("group2/int32", 0) == 1234567 );
+
     config->DeleteAll();
 }