From eb33447c3986417690acbc5733efeeebf67244d3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 9 Mar 2021 18:53:46 +0100 Subject: [PATCH] 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 | 6 ++ src/msw/regconf.cpp | 115 +++++++++------------------------------ tests/config/regconf.cpp | 4 ++ 3 files changed, 36 insertions(+), 89 deletions(-) diff --git a/include/wx/msw/regconf.h b/include/wx/msw/regconf.h index 0082c139a6..705dafdfa7 100644 --- a/include/wx/msw/regconf.h +++ b/include/wx/msw/regconf.h @@ -88,6 +88,12 @@ protected: return self->m_keyLocal; } + // Type-independent implementation of Do{Read,Write}Foo(). + template + bool DoReadValue(const wxString& key, T* pValue) const; + template + 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; diff --git a/src/msw/regconf.cpp b/src/msw/regconf.cpp index eeb3c90639..5ea2409164 100644 --- a/src/msw/regconf.cpp +++ b/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 +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)) || - (bQueryGlobal && TryGetValue(m_keyGlobal, path.Name(), *pStr)) ) { + if ( (m_keyLocal.Exists() && TryGetValue(LocalKey(), path.Name(), pValue)) || + (bQueryGlobal && TryGetValue(m_keyGlobal, path.Name(), pValue)) ) { return true; } return false; } -// this exactly reproduces the string version above except for ExpandEnvVars(), -// we really should avoid this code duplication somehow... +bool wxRegConfig::DoReadString(const wxString& key, wxString *pStr) const +{ + return DoReadValue(key, pStr); +} bool wxRegConfig::DoReadLong(const wxString& key, long *plResult) const { - wxCHECK_MSG( plResult, false, wxT("wxRegConfig::Read(): NULL param") ); - - 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; + return DoReadValue(key, plResult); } #if wxUSE_BASE64 bool wxRegConfig::DoReadBinary(const wxString& key, wxMemoryBuffer *buf) const { - wxCHECK_MSG( buf, false, wxT("wxRegConfig::Read(): NULL param") ); - - 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; + return DoReadValue(key, buf); } #endif // wxUSE_BASE64 -bool wxRegConfig::DoWriteString(const wxString& key, const wxString& szValue) +template +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); - - if ( IsImmutable(path.Name()) ) { - wxLogError(wxT("Can't change immutable entry '%s'."), path.Name().c_str()); - return false; - } - - return LocalKey().SetValue(path.Name(), lValue); + return DoWriteValue(key, lValue); } #if wxUSE_BASE64 bool wxRegConfig::DoWriteBinary(const wxString& key, const wxMemoryBuffer& buf) { - wxConfigPathChanger path(this, key); - - if ( IsImmutable(path.Name()) ) { - wxLogError(wxT("Can't change immutable entry '%s'."), path.Name().c_str()); - return false; - } - - return LocalKey().SetValue(path.Name(), buf); + return DoWriteValue(key, buf); } #endif // wxUSE_BASE64 diff --git a/tests/config/regconf.cpp b/tests/config/regconf.cpp index 623f503918..ab012cb544 100644 --- a/tests/config/regconf.cpp +++ b/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(); }