[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: readEntry and booleans
From:       Thomas Braxton <brax108 () cox ! net>
Date:       2006-01-05 20:14:44
Message-ID: 200601051414.44564.brax108 () cox ! net
[Download RAW message or body]

On Thursday 05 January 2006 06:24, David Faure wrote:
> The point is that people who compile with debug are willing to debug - so a
> crash is fine - and people who get release binaries are not willing to
> debug, so for them it's better to just try to cope with the problem as much
> as we can without bothering the user. A kdWarning helps in case the user
> reports some problem to us though.

How about this patch? I define kcbError depending on NDEBUG to 
kdWarning/kdFatal. gcc defines NDEBUG when it's not in debug mode, other 
compilers do the same, right? So for release builds it should just print a 
warning.

In the template code I could probably change it to a compile time concept 
check, but it's been a while since I've done any meta-programming, so it 
might take a few days while I get back up to speed. I think the concept check 
would be a good idea because it would catch most errors at compile time, 
where the programmer could fix them before they got out to users.

The only one I can't do much about is writing a QVariantList, because the 
programmer must ensure that only types that can be converted to string are 
passed in. Otherwise we would have to write a full encoder/decoder for any 
type that we want to be able to pass in a QVariantList. This would mean 
reading/writing lists inside lists and other nasties, sounds like it would be 
a PITA for something most applications probably wouldn't use.

Thomas

["diff" (text/x-diff)]

Index: kconfigbase.cpp
===================================================================
--- kconfigbase.cpp	(revision 494602)
+++ kconfigbase.cpp	(working copy)
@@ -1251,8 +1251,11 @@
     case QVariant::String:
       writeEntry( pKey, prop.toString(), pFlags );
       return;
+    case QVariant::List:
+      kcbError(!prop.canConvert(QVariant::StringList))
+        << "not all types in \"" << pKey << "\" can convert to QString,"
+           " information will be lost" << endl;
     case QVariant::StringList:
-    case QVariant::List:
       writeEntry( pKey, prop.toStringList(), ',', pFlags );
       return;
     case QVariant::ByteArray: {
Index: kconfigbase.h
===================================================================
--- kconfigbase.h	(revision 494602)
+++ kconfigbase.h	(working copy)
@@ -25,6 +25,7 @@
 
 #include <qvariant.h>
 #include <kdelibs_export.h>
+#include <kdebug.h>
 
 template <typename KT, typename KV> class QMap;
 class QString;
@@ -1502,9 +1503,20 @@
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( KConfigBase::WriteConfigFlags )
 
+#ifdef NDEBUG
+#define kcbError kdWarning
+#else
+#define kcbError kdFatal
+#endif
+
 template <typename T>
 QList<T> KConfigBase::readEntry( const char* pKey, const QList<T>& aDefault) const
 {
+  QVariant::Type wanted = QVariant(T()).type();
+  kcbError(!QVariant(QVariant::String).canConvert(wanted))
+    << "QString cannot convert to \"" << QVariant::typeToName(wanted)
+    << "\" information will be lost" << endl;
+
   if (!hasKey(pKey))
     return aDefault;
 
@@ -1518,8 +1530,11 @@
 
   QList<T> list;
   if (!vList.isEmpty()) {
-    foreach (QVariant aValue, vList)
+    foreach (QVariant aValue, vList) {
+      kcbError(!aValue.convert(wanted)) << "conversion to "
+        << QVariant::typeToName(wanted) << " information has been lost" << endl;
       list.append( qvariant_cast<T>(aValue) );
+    }
   }
 
   return list;
@@ -1530,9 +1545,17 @@
                               WriteConfigFlags pFlags )
 {
   QVariantList vList;
-  foreach(T aValue, rValue)
-    vList.append(aValue);
 
+  if (!rValue.isEmpty()) {
+    QVariant dummy = rValue.first();
+    kcbError(!dummy.canConvert(QVariant::String))
+       << QVariant::typeToName(dummy.type())
+       << " cannot convert to QString information will be lost" << endl;
+
+    foreach(T aValue, rValue)
+      vList.append(aValue);
+  }
+
   writeEntry( pKey, QVariant(vList), pFlags );
 }
 
Index: tests/kconfigtest.cpp
===================================================================
--- tests/kconfigtest.cpp	(revision 494602)
+++ tests/kconfigtest.cpp	(working copy)
@@ -24,6 +24,8 @@
 #include <kconfig.h>
 #include <kdebug.h>
 
+#define CRASH_ON_VARIANTLIST2 0
+
 QTTEST_KDEMAIN( KConfigTest, NoGUI )
 
 #define BOOLENTRY1 true
@@ -45,6 +47,7 @@
 #define COLORENTRY QColor("steelblue")
 #define FONTENTRY QFont("Times", 16, QFont::Normal)
 #define VARIANTLISTENTRY (QVariantList() << true << false << QString("joe") << 10023)
+#define VARIANTLISTENTRY2 (QVariantList() << POINTENTRY << SIZEENTRY)
 
 void KConfigTest::initTestCase()
 {
@@ -88,6 +91,11 @@
   sc.writeEntry( "listOfByteArraysEntry1", BYTEARRAYLISTENTRY1 );
   sc.writeEntry( "stringListEntry", STRINGLISTENTRY );
   sc.writeEntry( "variantListEntry", VARIANTLISTENTRY );
+
+#if CRASH_ON_VARIANTLIST2
+  // if debugging this _should_ cause a crash, otherwise a warning
+  sc.writeEntry( "variantListEntry2", VARIANTLISTENTRY2 );
+#endif
   sc.sync();
 }
 


[prev in list] [next in list] [prev in thread] [next in thread]