gnucash master: Implement KvpValue in c++ using boost::variant

John Ralls jralls at code.gnucash.org
Tue Oct 7 11:08:32 EDT 2014


Updated	 via  https://github.com/Gnucash/gnucash/commit/8a7f426f (commit)
	from  https://github.com/Gnucash/gnucash/commit/c60af70e (commit)



commit 8a7f426f3b37def46f5ecccc05bdb5b459791737
Author: lmat <dartme18 at gmail.com>
Date:   Fri Sep 19 16:35:03 2014 -0400

    Implement KvpValue in c++ using boost::variant
    
    KvpValue is now instantiated as a boost::variant and passed around as an opaque
    pointer in C. The C interface is basically unchanged and a c++ interface exists
    in kvp-value.hpp
    
    The c++ implementation for KvpValue is called KvpValueImpl and is in kvp-value.cpp.
    We don't use structured exception handling at this point, so c++ functions are
    marked 'noexcept'.
    
    The logic is within the c++ implementations. C wrapper functions do little besides
    ensure that the pointer is not nullptr before calling into c++.
    The logic in kvp_value_glist_to_string was moved to the c++ class. It's an
    implementation detail, but unfortunately, it was being exposed through a pointer,
    so I had to modify the test just a bit to not use it directly.
    
    In order to work around what seems to be a bug in an Apple compiler, it was decided
    to create an header file private to kvp-value and kvp_frame that holds the definition
    of KvpFrame so that it was visible to both translation units.

diff --git a/src/engine/engine.i b/src/engine/engine.i
index 8fe362f..bf0453c 100644
--- a/src/engine/engine.i
+++ b/src/engine/engine.i
@@ -238,7 +238,7 @@ Account * gnc_book_get_template_root(QofBook *book);
 
 void gnc_kvp_frame_delete_at_path(KvpFrame *frame, GSList *key_path);
 void kvp_frame_set_slot_path_gslist(
-   KvpFrame *frame, const KvpValue *new_value, GSList *key_path);
+   KvpFrame *frame, KvpValue *new_value, GSList *key_path);
 KvpValue * kvp_frame_get_slot_path_gslist (KvpFrame *frame, GSList *key_path);
 
 %clear GSList *key_path;
diff --git a/src/libqof/qof/Makefile.am b/src/libqof/qof/Makefile.am
index a1e3552..fc2d065 100644
--- a/src/libqof/qof/Makefile.am
+++ b/src/libqof/qof/Makefile.am
@@ -24,10 +24,11 @@ AM_CPPFLAGS = \
 
 libgnc_qof_la_SOURCES =  \
    gnc-date.cpp        \
-   gnc-numeric.cpp   \
+   gnc-numeric.cpp     \
    guid.cpp            \
    kvp-util.cpp        \
    kvp_frame.cpp       \
+   kvp-value.cpp       \
    qofbackend.cpp      \
    qofbook.cpp         \
    qofchoice.cpp       \
diff --git a/src/libqof/qof/kvp-util.cpp b/src/libqof/qof/kvp-util.cpp
index ad655f6..33f88ff 100644
--- a/src/libqof/qof/kvp-util.cpp
+++ b/src/libqof/qof/kvp-util.cpp
@@ -1,5 +1,5 @@
 /********************************************************************\
- * kvp_util.c -- misc odd-job kvp utils                             *
+ * kvp_util.cpp -- misc odd-job kvp utils                           *
  * Copyright (C) 2001 Linas Vepstas <linas at linas.org>               *
  *                                                                  *
  * This program is free software; you can redistribute it and/or    *
diff --git a/src/libqof/qof/kvp-value.cpp b/src/libqof/qof/kvp-value.cpp
new file mode 100644
index 0000000..771f0be
--- /dev/null
+++ b/src/libqof/qof/kvp-value.cpp
@@ -0,0 +1,322 @@
+/********************************************************************\
+ * kvp-value.cpp -- Implements a key-value frame system             *
+ * This program is free software; you can redistribute it and/or    *
+ * modify it under the terms of the GNU General Public License as   *
+ * published by the Free Software Foundation; either version 2 of   *
+ * the License, or (at your option) any later version.              *
+ *                                                                  *
+ * This program is distributed in the hope that it will be useful,  *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of   *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the    *
+ * GNU General Public License for more details.                     *
+ *                                                                  *
+ * You should have received a copy of the GNU General Public License*
+ * along with this program; if not, contact:                        *
+ *                                                                  *
+ * Free Software Foundation           Voice:  +1-617-542-5942       *
+ * 51 Franklin Street, Fifth Floor    Fax:    +1-617-542-2652       *
+ * Boston, MA  02110-1301,  USA       gnu at gnu.org                   *
+ *                                                                  *
+\********************************************************************/
+
+#include "kvp-value.hpp"
+#include <cmath>
+#include <sstream>
+#include <iomanip>
+#include "kvp_frame-p.hpp"
+
+KvpValueImpl::KvpValueImpl(KvpValueImpl const & other) noexcept
+{
+    if (other.datastore.type() == typeid(gchar *))
+        this->datastore = g_strdup(other.get<gchar *>());
+    else if (other.datastore.type() == typeid(GncGUID*))
+        this->datastore = guid_copy(other.get<GncGUID *>());
+    else if (other.datastore.type() == typeid(GList*))
+        this->datastore = kvp_glist_copy(other.get<GList *>());
+    else if (other.datastore.type() == typeid(KvpFrame*))
+        this->datastore = kvp_frame_copy(other.get<KvpFrame *>());
+    else
+        this->datastore = other.datastore;
+}
+
+KvpValueImpl *
+KvpValueImpl::add(KvpValueImpl * val) noexcept
+{
+    /* If already a glist here, just append */
+    if (this->datastore.type() == typeid(GList*))
+    {
+        GList * list = boost::get<GList*>(datastore);
+        datastore = g_list_append (list, val);
+        return this;
+    }
+    /* If some other value, convert it to a glist */
+    GList *list = nullptr;
+
+    list = g_list_append (list, this);
+    list = g_list_append (list, val);
+    return new KvpValueImpl(list);
+}
+
+KvpValueType
+KvpValueImpl::get_type() const noexcept
+{
+    if (datastore.type() == typeid(int64_t))
+        return KvpValueType::KVP_TYPE_GINT64;
+    else if (datastore.type() == typeid(double))
+        return KvpValueType::KVP_TYPE_DOUBLE;
+    else if (datastore.type() == typeid(gnc_numeric))
+        return KvpValueType::KVP_TYPE_NUMERIC;
+    else if (datastore.type() == typeid(gchar *))
+        return KvpValueType::KVP_TYPE_STRING;
+    else if (datastore.type() == typeid(GncGUID *))
+        return KvpValueType::KVP_TYPE_GUID;
+    else if (datastore.type() == typeid(Timespec))
+        return KvpValueType::KVP_TYPE_TIMESPEC;
+    else if (datastore.type() == typeid(GList *))
+        return KvpValueType::KVP_TYPE_GLIST;
+    else if (datastore.type() == typeid(KvpFrame *))
+        return KvpValueType::KVP_TYPE_FRAME;
+    else if (datastore.type() == typeid(GDate))
+        return KvpValueType::KVP_TYPE_GDATE;
+
+    return KVP_TYPE_INVALID;
+}
+
+KvpFrame *
+KvpValueImpl::replace_frame_nc (KvpFrame * new_value) noexcept
+{
+    if (datastore.type() != typeid(KvpFrame *))
+        return {};
+    auto ret = boost::get<KvpFrame *>(datastore);
+    datastore = new_value;
+    return ret;
+}
+
+GList *
+KvpValueImpl::replace_glist_nc (GList * new_value) noexcept
+{
+    if (datastore.type() != typeid(GList *))
+        return {};
+    auto ret = boost::get<GList *>(datastore);
+    datastore = new_value;
+    return ret;
+}
+
+struct to_string_visitor : boost::static_visitor<void>
+{
+    std::ostringstream & output;
+
+    to_string_visitor(std::ostringstream & val) : output(val){}
+
+    void operator()(int64_t val)
+    {
+        output << "KVP_VALUE_GINT64(" << val << ")";
+    }
+
+    void operator()(KvpFrame * val)
+    {
+        auto tmp1 = kvp_frame_to_string(val);
+        output << "KVP_VALUE_FRAME(";
+        if (tmp1)
+        {
+            output << tmp1;
+            g_free(tmp1);
+        }
+        output << ")";
+    }
+
+    void operator()(GDate val)
+    {
+        output << "KVP_VALUE_GDATE(";
+        output << std::setw(4) << g_date_get_year(&val) << '-';
+        output << std::setw(2) << g_date_get_month(&val) << '-';
+        output << std::setw(2) << g_date_get_day(&val) << ')';
+    }
+
+    void operator()(GList * val)
+    {
+        output << "KVP_VALUE_GLIST(";
+        output << "[ ";
+
+        /*Since val is passed by value, we can modify it*/
+        for (;val; val = val->next)
+        {
+            gchar *tmp3;
+            auto realvalue = static_cast<const KvpValue *>(val->data);
+            tmp3 = realvalue->to_string();
+            output << ' ';
+            if (tmp3)
+            {
+                output << tmp3;
+                g_free(tmp3);
+            }
+            output << ',';
+        }
+
+        output << " ]";
+        output << ")";
+    }
+
+    void operator()(Timespec val)
+    {
+        char tmp1[40];
+        gnc_timespec_to_iso8601_buff (val, tmp1);
+        output << "KVP_VALUE_TIMESPEC(" << tmp1 << ")";
+    }
+
+    void operator()(gnc_numeric val)
+    {
+        auto tmp1 = gnc_numeric_to_string(val);
+        output << "KVP_VALUE_NUMERIC(";
+        if (tmp1)
+        {
+            output << tmp1;
+            g_free(tmp1);
+        }
+        output << ")";
+    }
+
+    void operator()(GncGUID * val)
+    {
+        char guidstr[GUID_ENCODING_LENGTH+1];
+        output << "KVP_VALUE_GUID(";
+        if (val)
+        {
+            guid_to_string_buff(val,guidstr);
+            output << guidstr;
+        }
+        output << ")";
+    }
+
+    void operator()(char * val)
+    {
+        output << "KVP_VALUE_STRING(" << val << ")";
+    }
+
+    void operator()(double val)
+    {
+        output << "KVP_VALUE_DOUBLE(" << val << ")";
+    }
+};
+
+char *
+KvpValueImpl::to_string() const noexcept
+{
+    std::ostringstream ret;
+    to_string_visitor visitor {ret};
+    boost::apply_visitor(visitor, datastore);
+
+    /*We still use g_strdup since the return value will be freed by g_free*/
+
+    return g_strdup(ret.str().c_str());
+}
+
+struct compare_visitor : boost::static_visitor<int>
+{
+    template <typename T, typename U>
+    int operator()( T& one, U& two) const
+    {
+        throw std::invalid_argument{"You may not compare objects of different type."};
+    }
+
+    template <typename T>
+    int operator()( T & one, T & two) const
+    {
+        /*This will work for any type that implements < and ==.*/
+        if ( one < two )
+            return -1;
+        else if (one == two)
+            return 0;
+        else
+            return 1;
+    }
+};
+template <> int compare_visitor::operator()(char * const & one, char * const & two) const
+{
+    return strcmp(one, two);
+}
+template <> int compare_visitor::operator()(gnc_numeric const & one, gnc_numeric const & two) const
+{
+    return gnc_numeric_compare(one, two);
+}
+template <> int compare_visitor::operator()(GncGUID * const & one, GncGUID * const & two) const
+{
+    return guid_compare(one, two);
+}
+template <> int compare_visitor::operator()(Timespec const & one, Timespec const & two) const
+{
+    return timespec_cmp(&one,&two);
+}
+template <> int compare_visitor::operator()(GDate const & one, GDate const & two) const
+{
+    return g_date_compare(&one,&two);
+}
+template <> int compare_visitor::operator()(GList * const & one, GList * const & two) const
+{
+    return kvp_glist_compare(one, two);
+}
+template <> int compare_visitor::operator()(KvpFrame * const & one, KvpFrame * const & two) const
+{
+    return kvp_frame_compare(one, two);
+}
+template <> int compare_visitor::operator()(double const & one, double const & two) const
+{
+    if (std::isnan(one) && std::isnan(two))
+        return 0;
+    if (one < two) return -1;
+    /*perhaps we should have tolerance here?*/
+    if (one == two) return 0;
+    return 1;
+}
+
+int compare(const KvpValueImpl & one, const KvpValueImpl & two) noexcept
+{
+    auto type1 = one.get_type();
+    auto type2 = two.get_type();
+
+    if (type1 != type2)
+        return type1 < type2 ? -1 : 1;
+
+    compare_visitor comparer;
+    return boost::apply_visitor(comparer, one.datastore, two.datastore);
+}
+
+int
+compare(const KvpValueImpl * one, const KvpValueImpl * two) noexcept
+{
+    if (one == two) return 0;
+    if (one && !two) return 1;
+    if (!one && two) return -1;
+    return compare(*one, *two);
+}
+
+template <typename T> void
+delete_value(T & value)
+{
+    /*do nothing*/
+}
+template <> void
+delete_value(GList * & value)
+{
+    kvp_glist_delete(value);
+}
+template <> void
+delete_value(gchar * & value)
+{
+    g_free(value);
+}
+template <> void
+delete_value(GncGUID * & value)
+{
+    guid_free(value);
+}
+template <> void
+delete_value(KvpFrame * & value)
+{
+    kvp_frame_delete(value);
+}
+
+KvpValueImpl::~KvpValueImpl() noexcept
+{
+    delete_value(datastore);
+}
diff --git a/src/libqof/qof/kvp-value.hpp b/src/libqof/qof/kvp-value.hpp
new file mode 100644
index 0000000..e56a369
--- /dev/null
+++ b/src/libqof/qof/kvp-value.hpp
@@ -0,0 +1,127 @@
+#ifndef gnc_kvp_value_type
+#define gnc_kvp_value_type
+
+/********************************************************************\
+ * kvp-value.hpp -- Implements a key-value frame system             *
+ * This program is free software; you can redistribute it and/or    *
+ * modify it under the terms of the GNU General Public License as   *
+ * published by the Free Software Foundation; either version 2 of   *
+ * the License, or (at your option) any later version.              *
+ *                                                                  *
+ * This program is distributed in the hope that it will be useful,  *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of   *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the    *
+ * GNU General Public License for more details.                     *
+ *                                                                  *
+ * You should have received a copy of the GNU General Public License*
+ * along with this program; if not, contact:                        *
+ *                                                                  *
+ * Free Software Foundation           Voice:  +1-617-542-5942       *
+ * 51 Franklin Street, Fifth Floor    Fax:    +1-617-542-2652       *
+ * Boston, MA  02110-1301,  USA       gnu at gnu.org                   *
+ *                                                                  *
+\********************************************************************/
+
+extern "C"
+{
+#include "config.h"
+#include "qof.h"
+}
+
+#include <boost/type_traits/is_nothrow_move_assignable.hpp>
+#include <boost/variant.hpp>
+
+struct KvpValueImpl
+{
+    /**
+     * Performs a deep copy
+     */
+    KvpValueImpl(KvpValueImpl const &) noexcept;
+
+    template <typename T>
+    KvpValueImpl(T) noexcept;
+
+    /**
+     * Performs a deep delete.
+     *
+     * The contents of this KvpValueImpl are also deleted.
+     */
+    ~KvpValueImpl() noexcept;
+
+    /**
+     * Replaces the frame within this KvpValueImpl.
+     *
+     * If this KvpValueImpl doesn't contain a KvpFrame, nullptr
+     * is returned. Otherwise, the old KvpFrame * is returned.
+     */
+    KvpFrame * replace_frame_nc (KvpFrame *) noexcept;
+
+    /**
+     * Replaces the glist within this KvpValueImpl.
+     *
+     * If this KvpValueImpl doesn't contain a GList, nullptr
+     * is returned. Otherwise, the old GList * is returned.
+     */
+    GList * replace_glist_nc (GList *) noexcept;
+
+    /**
+     * Adds another value to this KvpValueImpl.
+     *
+     * If this KvpValueImpl represents a collection (GList),
+     * the new value is added to the collection and this
+     * is returned.
+     *
+     * Otherwise, a new KvpValueImpl representing a collection
+     * is created, this and the new value are added to it,
+     * and it is returned.
+     */
+    KvpValueImpl * add (KvpValueImpl *) noexcept;
+
+    KvpValueType get_type() const noexcept;
+
+    char * to_string() const noexcept;
+
+    template <typename T>
+    T get() const noexcept;
+
+    template <typename T>
+    void set(T) noexcept;
+
+    friend int compare(const KvpValueImpl &, const KvpValueImpl &) noexcept;
+
+    private:
+    boost::variant<
+        int64_t,
+        double,
+        gnc_numeric,
+        char *,
+        GncGUID *,
+        Timespec,
+        GList *,
+        boost::recursive_wrapper<KvpFrame *>,
+        GDate> datastore;
+};
+
+int
+compare(const KvpValueImpl *, const KvpValue *) noexcept;
+
+template <typename T>
+KvpValueImpl::KvpValueImpl(T newvalue) noexcept:
+    datastore(newvalue)
+{
+}
+
+template <typename T> T
+KvpValueImpl::get() const noexcept
+{
+    if (this->datastore.type() != typeid(T)) return {};
+    return boost::get<T>(datastore);
+}
+
+template <typename T> void
+KvpValueImpl::set(T val) noexcept
+{
+    this->datastore = val;
+}
+
+#endif
diff --git a/src/libqof/qof/kvp_frame-p.hpp b/src/libqof/qof/kvp_frame-p.hpp
new file mode 100644
index 0000000..3678083
--- /dev/null
+++ b/src/libqof/qof/kvp_frame-p.hpp
@@ -0,0 +1,14 @@
+#ifndef kvp_frame_p_header
+#define kvp_frame_p_header
+
+/* Note that we keep the keys for this hash table in the
+ * qof_string_cache, as it is very likely we will see the
+ * same keys over and over again  */
+
+struct _KvpFrame
+{
+    GHashTable  * hash;
+};
+
+
+#endif
diff --git a/src/libqof/qof/kvp_frame.cpp b/src/libqof/qof/kvp_frame.cpp
index 5bd0b27..db20f9c 100644
--- a/src/libqof/qof/kvp_frame.cpp
+++ b/src/libqof/qof/kvp_frame.cpp
@@ -1,5 +1,5 @@
 /********************************************************************
- * kvp_frame.c -- Implements a key-value frame system               *
+ * kvp_frame.cpp -- Implements a key-value frame system             *
  * Copyright (C) 2000 Bill Gribble                                  *
  * Copyright (C) 2001,2003 Linas Vepstas <linas at linas.org>          *
  *                                                                  *
@@ -22,50 +22,21 @@
  *                                                                  *
  ********************************************************************/
 
-#ifdef __cplusplus
 extern "C"
 {
-#endif
-
 #include "config.h"
-
+#include "qof.h"
 #include <glib.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
-#include <math.h>
-
-#ifdef __cplusplus
 }
-#endif
 
-#include "qof.h"
+#include <typeinfo>
+#include <iostream>
+#include "kvp-value.hpp"
+#include "kvp_frame-p.hpp"
 
-/* Note that we keep the keys for this hash table in the
- * qof_string_cache, as it is very likely we will see the
- * same keys over and over again  */
-
-struct _KvpFrame
-{
-    GHashTable  * hash;
-};
-
-struct _KvpValue
-{
-    KvpValueType type;
-    union
-    {
-        gint64 int64;
-        double dbl;
-        gnc_numeric numeric;
-        gchar *str;
-        GncGUID *guid;
-        Timespec timespec;
-        GList *list;
-        KvpFrame *frame;
-        GDate gdate;
-    } value;
-};
 
 /* This static indicates the debugging module that this .o belongs to.  */
 static QofLogModule log_module = QOF_MOD_KVP;
@@ -110,7 +81,7 @@ static void
 kvp_frame_delete_worker(gpointer key, gpointer value, G_GNUC_UNUSED gpointer user_data)
 {
     qof_string_cache_remove(key);
-    kvp_value_delete((KvpValue *)value);
+    kvp_value_delete(static_cast<KvpValue *>(value));
 }
 
 void
@@ -389,8 +360,6 @@ get_trailer_or_null (const KvpFrame * frame, const char * key_path, char **end_k
     return frame;
 }
 
-/* ============================================================ */
-
 void
 kvp_frame_set_gint64(KvpFrame * frame, const char * path, gint64 ival)
 {
@@ -514,41 +483,22 @@ kvp_frame_replace_value_nc (KvpFrame * frame, const char * key_path,
     return old_value;
 }
 
-/* ============================================================ */
-
 static KvpFrame *
 kvp_frame_add_value_nc(KvpFrame * frame, const char * path, KvpValue *value)
 {
     char *key = NULL;
-    KvpValue *oldvalue;
+    KvpValueImpl * oldvalue;
     KvpFrame* orig_frame = frame;
 
     frame = (KvpFrame *) get_trailer_or_null (frame, path, &key);
-    oldvalue = kvp_frame_get_slot (frame, key);
+    oldvalue = static_cast<KvpValueImpl *> (kvp_frame_get_slot (frame, key));
+    auto newvalue = static_cast<KvpValueImpl *> (value);
 
     ENTER ("old frame=%s", kvp_frame_to_string(frame));
     if (oldvalue)
     {
-        /* If already a glist here, just append */
-        if (KVP_TYPE_GLIST == oldvalue->type)
-        {
-            GList * vlist = oldvalue->value.list;
-            vlist = g_list_append (vlist, value);
-            oldvalue->value.list = vlist;
-        }
-        else
-            /* If some other value, convert it to a glist */
-        {
-            KvpValue *klist;
-            GList *vlist = NULL;
-
-            vlist = g_list_append (vlist, oldvalue);
-            vlist = g_list_append (vlist, value);
-            klist = kvp_value_new_glist_nc (vlist);
-
-            kvp_frame_replace_slot_nc (frame, key, klist);
-        }
-        LEAVE ("new frame=%s", kvp_frame_to_string(frame));
+        kvp_frame_replace_slot_nc (frame, key, oldvalue->add (newvalue));
+        LEAVE ("new frame=%s", kvp_frame_to_string (frame));
         return frame;
     }
 
@@ -573,7 +523,7 @@ kvp_frame_add_frame_nc(KvpFrame * frame, const char * path, KvpFrame *fr)
 
 void
 kvp_frame_set_slot(KvpFrame * frame, const char * slot,
-                   const KvpValue * value)
+                   KvpValue * value)
 {
     KvpValue *new_value = NULL;
 
@@ -601,7 +551,7 @@ kvp_frame_get_slot(const KvpFrame * frame, const char * slot)
 {
     KvpValue *v;
     if (!frame) return NULL;
-    if (!frame->hash) return NULL;  /* Error ... */
+    if (!frame->hash) return NULL;
     v = static_cast<KvpValue*>(g_hash_table_lookup(frame->hash, slot));
     return v;
 }
@@ -610,7 +560,7 @@ kvp_frame_get_slot(const KvpFrame * frame, const char * slot)
 
 void
 kvp_frame_set_slot_path (KvpFrame *frame,
-                         const KvpValue *new_value,
+                         KvpValue * new_value,
                          const char *first_key, ...)
 {
     va_list ap;
@@ -661,7 +611,7 @@ kvp_frame_set_slot_path (KvpFrame *frame,
 
 void
 kvp_frame_set_slot_path_gslist (KvpFrame *frame,
-                                const KvpValue *new_value,
+                                KvpValue * new_value,
                                 GSList *key_path)
 {
     if (!frame || !key_path) return;
@@ -878,7 +828,7 @@ kvp_glist_copy(const GList * list)
     /* This step deep-copies the values */
     for (lptr = retval; lptr; lptr = lptr->next)
     {
-        lptr->data = kvp_value_copy(static_cast<KvpValue*>(lptr->data));
+        lptr->data = kvp_value_copy(static_cast<KvpValue *>(lptr->data));
     }
 
     return retval;
@@ -917,343 +867,189 @@ kvp_glist_compare(const GList * list1, const GList * list2)
  ********************************************************************/
 
 KvpValue *
-kvp_value_new_gint64(gint64 value)
+kvp_value_new_gint64(int64_t value)
 {
-    KvpValue * retval  = g_new0(KvpValue, 1);
-    retval->type        = KVP_TYPE_GINT64;
-    retval->value.int64 = value;
-    return retval;
+    return new KvpValueImpl{value};
 }
 
 KvpValue *
 kvp_value_new_double(double value)
 {
-    KvpValue * retval  = g_new0(KvpValue, 1);
-    retval->type        = KVP_TYPE_DOUBLE;
-    retval->value.dbl   = value;
-    return retval;
+    return new KvpValueImpl{value};
 }
 
 KvpValue *
 kvp_value_new_numeric(gnc_numeric value)
 {
-    KvpValue * retval    = g_new0(KvpValue, 1);
-    retval->type          = KVP_TYPE_NUMERIC;
-    retval->value.numeric = value;
-    return retval;
+    return new KvpValueImpl{value};
 }
 
 KvpValue *
 kvp_value_new_string(const char * value)
 {
-    KvpValue * retval;
-    if (!value) return NULL;
-
-    retval = g_new0(KvpValue, 1);
-    retval->type       = KVP_TYPE_STRING;
-    retval->value.str  = g_strdup(value);
-    return retval;
+    if (!value) return {};
+    return new KvpValueImpl{g_strdup(value)};
 }
 
 KvpValue *
 kvp_value_new_guid(const GncGUID * value)
 {
-    KvpValue * retval;
-    if (!value) return NULL;
-
-    retval = g_new0(KvpValue, 1);
-    retval->type       = KVP_TYPE_GUID;
-    retval->value.guid = g_new0(GncGUID, 1);
-    memcpy(retval->value.guid, value, sizeof(GncGUID));
-    return retval;
+    if (!value) return {};
+    return new KvpValueImpl{guid_copy(value)};
 }
 
 KvpValue *
 kvp_value_new_timespec(Timespec value)
 {
-    KvpValue * retval = g_new0(KvpValue, 1);
-    retval->type       = KVP_TYPE_TIMESPEC;
-    retval->value.timespec = value;
-    return retval;
+    return new KvpValueImpl{value};
 }
 
 KvpValue *
 kvp_value_new_gdate(GDate value)
 {
-    KvpValue * retval = g_new0(KvpValue, 1);
-    retval->type       = KVP_TYPE_GDATE;
-    retval->value.gdate = value;
-    return retval;
+    return new KvpValueImpl{value};
 }
 
 KvpValue *
 kvp_value_new_glist(const GList * value)
 {
-    KvpValue * retval;
-    if (!value) return NULL;
-
-    retval = g_new0(KvpValue, 1);
-    retval->type       = KVP_TYPE_GLIST;
-    retval->value.list = kvp_glist_copy(value);
-    return retval;
+    if (!value) return {};
+    return new KvpValueImpl{kvp_glist_copy(value)};
 }
 
 KvpValue *
 kvp_value_new_glist_nc(GList * value)
 {
-    KvpValue * retval;
-    if (!value) return NULL;
-
-    retval = g_new0(KvpValue, 1);
-    retval->type       = KVP_TYPE_GLIST;
-    retval->value.list = value;
-    return retval;
+    if (!value) return {};
+    return new KvpValueImpl{value};
 }
 
 KvpValue *
 kvp_value_new_frame(const KvpFrame * value)
 {
-    KvpValue * retval;
-    if (!value) return NULL;
-
-    retval  = g_new0(KvpValue, 1);
-    retval->type        = KVP_TYPE_FRAME;
-    retval->value.frame = kvp_frame_copy(value);
-    return retval;
+    if (!value) return {};
+    return new KvpValueImpl{kvp_frame_copy(value)};
 }
 
 KvpValue *
 kvp_value_new_frame_nc(KvpFrame * value)
 {
-    KvpValue * retval;
-    if (!value) return NULL;
-
-    retval  = g_new0(KvpValue, 1);
-    retval->type        = KVP_TYPE_FRAME;
-    retval->value.frame = value;
-    return retval;
+    if (!value) return {};
+    return new KvpValueImpl{value};
 }
 
 void
 kvp_value_delete(KvpValue * value)
 {
     if (!value) return;
-
-    switch (value->type)
-    {
-    case KVP_TYPE_STRING:
-        g_free(value->value.str);
-        break;
-    case KVP_TYPE_GUID:
-        g_free(value->value.guid);
-        break;
-    case KVP_TYPE_GLIST:
-        kvp_glist_delete(value->value.list);
-        break;
-    case KVP_TYPE_FRAME:
-        kvp_frame_delete(value->value.frame);
-        break;
-
-    default:
-        break;
-    }
-    g_free(value);
+    KvpValueImpl * realvalue {static_cast<KvpValueImpl *>(value)};
+    delete realvalue;
 }
 
 KvpValueType
-kvp_value_get_type(const KvpValue * value)
+kvp_value_get_type(const KvpValue * oldval)
 {
-    if (!value) return KVP_TYPE_INVALID;
-    return value->type;
+    if (!oldval) return KVP_TYPE_INVALID;
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(oldval)};
+    return value->get_type();
 }
 
-gint64
-kvp_value_get_gint64(const KvpValue * value)
+int64_t
+kvp_value_get_gint64(const KvpValue * ovalue)
 {
-    if (!value) return 0;
-    if (value->type == KVP_TYPE_GINT64)
-    {
-        return value->value.int64;
-    }
-    else
-    {
-        return 0;
-    }
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<int64_t>();
 }
 
 double
-kvp_value_get_double(const KvpValue * value)
+kvp_value_get_double(const KvpValue * ovalue)
 {
-    if (!value) return 0.0;
-    if (value->type == KVP_TYPE_DOUBLE)
-    {
-        return value->value.dbl;
-    }
-    else
-    {
-        return 0.0;
-    }
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<double>();
 }
 
 gnc_numeric
-kvp_value_get_numeric(const KvpValue * value)
+kvp_value_get_numeric(const KvpValue * ovalue)
 {
-    if (!value) return gnc_numeric_zero ();
-    if (value->type == KVP_TYPE_NUMERIC)
-    {
-        return value->value.numeric;
-    }
-    else
-    {
-        return gnc_numeric_zero ();
-    }
+    //if (!ovalue) return {}; The code depends on no segfault and zero being returned here.
+    if (!ovalue) return gnc_numeric_zero();
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<gnc_numeric>();
 }
 
 char *
-kvp_value_get_string(const KvpValue * value)
+kvp_value_get_string(const KvpValue * ovalue)
 {
-    if (!value) return NULL;
-    if (value->type == KVP_TYPE_STRING)
-    {
-        return value->value.str;
-    }
-    else
-    {
-        return NULL;
-    }
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<char*>();
 }
 
 GncGUID *
-kvp_value_get_guid(const KvpValue * value)
+kvp_value_get_guid(const KvpValue * ovalue)
 {
-    if (!value) return NULL;
-    if (value->type == KVP_TYPE_GUID)
-    {
-        return value->value.guid;
-    }
-    else
-    {
-        return NULL;
-    }
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<GncGUID*>();
 }
 
 Timespec
-kvp_value_get_timespec(const KvpValue * value)
-{
-    Timespec ts;
-    ts.tv_sec = 0;
-    ts.tv_nsec = 0;
-    if (!value) return ts;
-    if (value->type == KVP_TYPE_TIMESPEC)
-        return value->value.timespec;
-    else
-        return ts;
+kvp_value_get_timespec(const KvpValue * ovalue)
+{
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<Timespec>();
 }
 
 GDate
-kvp_value_get_gdate(const KvpValue * value)
+kvp_value_get_gdate(const KvpValue * ovalue)
 {
-    GDate date;
-    g_date_clear(&date, 1);
-    if (!value) return date;
-    if (value->type == KVP_TYPE_GDATE)
-        return value->value.gdate;
-    else
-        return date;
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<GDate>();
 }
 
 GList *
-kvp_value_get_glist(const KvpValue * value)
+kvp_value_get_glist(const KvpValue * ovalue)
 {
-    if (!value) return NULL;
-    if (value->type == KVP_TYPE_GLIST)
-    {
-        return value->value.list;
-    }
-    else
-    {
-        return NULL;
-    }
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<GList*>();
 }
 
 KvpFrame *
-kvp_value_get_frame(const KvpValue * value)
+kvp_value_get_frame(const KvpValue * ovalue)
 {
-    if (!value) return NULL;
-    if (value->type == KVP_TYPE_FRAME)
-    {
-        return value->value.frame;
-    }
-    else
-    {
-        return NULL;
-    }
+    if (!ovalue) return {};
+    const KvpValueImpl * value {static_cast<const KvpValueImpl *>(ovalue)};
+    return value->get<KvpFrame*>();
 }
 
 KvpFrame *
-kvp_value_replace_frame_nc(KvpValue *value, KvpFrame * newframe)
+kvp_value_replace_frame_nc(KvpValue * ovalue, KvpFrame * newframe)
 {
-    KvpFrame *oldframe;
-    if (!value) return NULL;
-    if (KVP_TYPE_FRAME != value->type) return NULL;
-
-    oldframe = value->value.frame;
-    value->value.frame = newframe;
-    return oldframe;
+    if (!ovalue) return {};
+    KvpValueImpl * value {static_cast<KvpValueImpl *>(ovalue)};
+    return value->replace_frame_nc (newframe);
 }
 
 GList *
-kvp_value_replace_glist_nc(KvpValue * value, GList *newlist)
+kvp_value_replace_glist_nc(KvpValue * ovalue, GList *newlist)
 {
-    GList *oldlist;
-    if (!value) return NULL;
-    if (KVP_TYPE_GLIST != value->type) return NULL;
-
-    oldlist = value->value.list;
-    value->value.list = newlist;
-    return oldlist;
+    if (!ovalue) return {};
+    KvpValueImpl * value {static_cast<KvpValueImpl *>(ovalue)};
+    return value->replace_glist_nc (newlist);
 }
 
-/* manipulators */
-
 KvpValue *
-kvp_value_copy(const KvpValue * value)
+kvp_value_copy(const KvpValue * ovalue)
 {
-    if (!value) return NULL;
-
-    switch (value->type)
-    {
-    case KVP_TYPE_GINT64:
-        return kvp_value_new_gint64(value->value.int64);
-        break;
-    case KVP_TYPE_DOUBLE:
-        return kvp_value_new_double(value->value.dbl);
-        break;
-    case KVP_TYPE_NUMERIC:
-        return kvp_value_new_gnc_numeric(value->value.numeric);
-        break;
-    case KVP_TYPE_STRING:
-        return kvp_value_new_string(value->value.str);
-        break;
-    case KVP_TYPE_GUID:
-        return kvp_value_new_guid(value->value.guid);
-        break;
-    case KVP_TYPE_GDATE:
-        return kvp_value_new_gdate(value->value.gdate);
-        break;
-    case KVP_TYPE_TIMESPEC:
-        return kvp_value_new_timespec(value->value.timespec);
-        break;
-    case KVP_TYPE_GLIST:
-        return kvp_value_new_glist(value->value.list);
-        break;
-    case KVP_TYPE_FRAME:
-        return kvp_value_new_frame(value->value.frame);
-        break;
-    default:
-	break;
-    }
-    return NULL;
+    if (!ovalue) return {};
+    auto value = static_cast<const KvpValueImpl *>(ovalue);
+    KvpValueImpl * ret = new KvpValueImpl(*value);
+    return static_cast<KvpValue *>(ret);
 }
 
 void
@@ -1270,65 +1066,12 @@ kvp_frame_for_each_slot(KvpFrame *f,
     g_hash_table_foreach(f->hash, (GHFunc) proc, data);
 }
 
-#ifdef _MSC_VER
-# define isnan _isnan
-#endif
-gint
-double_compare(double d1, double d2)
+int
+kvp_value_compare(const KvpValue * okva, const KvpValue * okvb)
 {
-    if (isnan(d1) && isnan(d2)) return 0;
-    if (d1 < d2) return -1;
-    if (d1 > d2) return 1;
-    return 0;
-}
-
-gint
-kvp_value_compare(const KvpValue * kva, const KvpValue * kvb)
-{
-    if (kva == kvb) return 0;
-    /* nothing is always less than something */
-    if (!kva && kvb) return -1;
-    if (kva && !kvb) return 1;
-
-    if (kva->type < kvb->type) return -1;
-    if (kva->type > kvb->type) return 1;
-
-    switch (kva->type)
-    {
-    case KVP_TYPE_GINT64:
-        if (kva->value.int64 < kvb->value.int64) return -1;
-        if (kva->value.int64 > kvb->value.int64) return 1;
-        return 0;
-        break;
-    case KVP_TYPE_DOUBLE:
-        return double_compare(kva->value.dbl, kvb->value.dbl);
-        break;
-    case KVP_TYPE_NUMERIC:
-        return gnc_numeric_compare (kva->value.numeric, kvb->value.numeric);
-        break;
-    case KVP_TYPE_STRING:
-        return strcmp(kva->value.str, kvb->value.str);
-        break;
-    case KVP_TYPE_GUID:
-        return guid_compare(kva->value.guid, kvb->value.guid);
-        break;
-    case KVP_TYPE_TIMESPEC:
-        return timespec_cmp(&(kva->value.timespec), &(kvb->value.timespec));
-        break;
-    case KVP_TYPE_GDATE:
-        return g_date_compare(&(kva->value.gdate), &(kvb->value.gdate));
-        break;
-    case KVP_TYPE_GLIST:
-        return kvp_glist_compare(kva->value.list, kvb->value.list);
-        break;
-    case KVP_TYPE_FRAME:
-        return kvp_frame_compare(kva->value.frame, kvb->value.frame);
-        break;
-    default:
-	break;
-    }
-    PERR ("reached unreachable code.");
-    return FALSE;
+    auto kva = static_cast<const KvpValueImpl *>(okva);
+    auto kvb = static_cast<const KvpValueImpl *>(okvb);
+    return compare(kva, kvb);
 }
 
 typedef struct
@@ -1386,30 +1129,12 @@ kvp_frame_compare(const KvpFrame *fa, const KvpFrame *fb)
     return(-status.compare);
 }
 
-static gchar*
-kvp_value_glist_to_string(const GList *list)
+char *
+kvp_value_to_string(const KvpValue * val)
 {
-    gchar *tmp1;
-    gchar *tmp2;
-    const GList *cursor;
-
-    tmp1 = g_strdup_printf("[ ");
-
-    for (cursor = list; cursor; cursor = cursor->next)
-    {
-        gchar *tmp3;
-
-        tmp3 = kvp_value_to_string((KvpValue *)cursor->data);
-        tmp2 = g_strdup_printf("%s %s,", tmp1, tmp3 ? tmp3 : "");
-        g_free(tmp1);
-        g_free(tmp3);
-        tmp1 = tmp2;
-    }
-
-    tmp2 = g_strdup_printf("%s ]", tmp1);
-    g_free(tmp1);
-
-    return tmp2;
+    if (!val) return g_strdup("");
+    auto realval = static_cast<const KvpValueImpl *>(val);
+    return realval->to_string();
 }
 
 /* struct for kvp frame static funtion testing*/
@@ -1425,7 +1150,6 @@ void init_static_test_pointers( void );
 #endif
 
 KvpFrame* ( *p_get_trailer_make )( KvpFrame *frame, const char *key_path, char **end_key );
-gchar* ( *p_kvp_value_glist_to_string )( const GList *list );
 KvpFrame* ( *p_get_or_make )( KvpFrame *fr, const char * key );
 const KvpFrame* ( *p_kvp_frame_get_frame_or_null_slash_trash )( const KvpFrame *frame, char *key_path );
 const KvpFrame* ( *p_get_trailer_or_null )( const KvpFrame * frame, const char * key_path, char **end_key );
@@ -1434,7 +1158,6 @@ void
 init_static_test_pointers( void )
 {
     p_get_trailer_make = get_trailer_make;
-    p_kvp_value_glist_to_string = kvp_value_glist_to_string;
     p_get_or_make = get_or_make;
     p_kvp_frame_get_frame_or_null_slash_trash = kvp_frame_get_frame_or_null_slash_trash;
     p_get_trailer_or_null = get_trailer_or_null;
@@ -1442,80 +1165,6 @@ init_static_test_pointers( void )
 
 /* ----- */
 
-gchar*
-kvp_value_to_string(const KvpValue *val)
-{
-    gchar *tmp1;
-    gchar *tmp2;
-    const gchar *ctmp;
-
-    g_return_val_if_fail(val, NULL);
-
-    switch (kvp_value_get_type(val))
-    {
-    case KVP_TYPE_GINT64:
-        return g_strdup_printf("KVP_VALUE_GINT64(%" G_GINT64_FORMAT ")",
-                               kvp_value_get_gint64(val));
-        break;
-
-    case KVP_TYPE_DOUBLE:
-        return g_strdup_printf("KVP_VALUE_DOUBLE(%g)",
-                               kvp_value_get_double(val));
-        break;
-
-    case KVP_TYPE_NUMERIC:
-        tmp1 = gnc_numeric_to_string(kvp_value_get_numeric(val));
-        tmp2 = g_strdup_printf("KVP_VALUE_NUMERIC(%s)", tmp1 ? tmp1 : "");
-        g_free(tmp1);
-        return tmp2;
-        break;
-
-    case KVP_TYPE_STRING:
-        tmp1 = kvp_value_get_string (val);
-        return g_strdup_printf("KVP_VALUE_STRING(%s)", tmp1 ? tmp1 : "");
-        break;
-
-    case KVP_TYPE_GUID:
-        gchar guidstr[GUID_ENCODING_LENGTH+1];
-        guid_to_string_buff(kvp_value_get_guid(val),guidstr);
-        tmp2 = g_strdup_printf("KVP_VALUE_GUID(%s)", guidstr);
-        return tmp2;
-        break;
-
-    case KVP_TYPE_TIMESPEC:
-        tmp1 = g_new0 (char, 40);
-        gnc_timespec_to_iso8601_buff (kvp_value_get_timespec (val), tmp1);
-        tmp2 = g_strdup_printf("KVP_VALUE_TIMESPEC(%s)", tmp1);
-        g_free(tmp1);
-        return tmp2;
-    break;
-
-    case KVP_TYPE_GLIST:
-        tmp1 = kvp_value_glist_to_string(kvp_value_get_glist(val));
-        tmp2 = g_strdup_printf("KVP_VALUE_GLIST(%s)", tmp1 ? tmp1 : "");
-        g_free(tmp1);
-        return tmp2;
-        break;
-
-    case KVP_TYPE_FRAME:
-        tmp1 = kvp_frame_to_string(kvp_value_get_frame(val));
-        tmp2 = g_strdup_printf("KVP_VALUE_FRAME(%s)", tmp1 ? tmp1 : "");
-        g_free(tmp1);
-        return tmp2;
-        break;
-
-    case KVP_TYPE_GDATE:
-        return g_strdup_printf("KVP_VALUE_GDATE(%04d-%02d-%02d)",
-                               g_date_get_year(&val->value.gdate),
-                               g_date_get_month(&val->value.gdate),
-                               g_date_get_day(&val->value.gdate));
-    default:
-	break;
-    }
-    g_assert(FALSE); /* must not be reached */
-    return g_strdup("");
-}
-
 static void
 kvp_frame_to_string_helper(gpointer key, gpointer value, gpointer data)
 {
@@ -1563,7 +1212,7 @@ kvp_frame_get_hash(const KvpFrame *frame)
     return frame->hash;
 }
 
-static GValue *gvalue_from_kvp_value (KvpValue*);
+static GValue *gvalue_from_kvp_value (KvpValue *);
 static KvpValue *kvp_value_from_gvalue (const GValue*);
 
 static void
@@ -1598,7 +1247,7 @@ gvalue_from_kvp_value (KvpValue *kval)
     if (kval == NULL) return NULL;
     val = g_slice_new0 (GValue);
 
-    switch (kval->type)
+    switch (kvp_value_get_type(kval))
     {
 	case KVP_TYPE_GINT64:
 	    g_value_init (val, G_TYPE_INT64);
diff --git a/src/libqof/qof/kvp_frame.h b/src/libqof/qof/kvp_frame.h
index 2bcd737..810e064 100644
--- a/src/libqof/qof/kvp_frame.h
+++ b/src/libqof/qof/kvp_frame.h
@@ -77,7 +77,7 @@ typedef struct _KvpFrame KvpFrame;
 
 /** A KvpValue is a union with possible types enumerated in the
  * KvpValueType enum. */
-typedef struct _KvpValue KvpValue;
+typedef struct KvpValueImpl KvpValue;
 
 /** \brief possible types in the union KvpValue
  * \todo : People have asked for boolean values,
@@ -347,7 +347,7 @@ You probably shouldn't be using these low-level routines
  *    this location, if any, is destroyed.
  */
 void          kvp_frame_set_slot(KvpFrame * frame,
-                                 const gchar * key, const KvpValue * value);
+                                 const gchar * key, KvpValue * value);
 /**
  * The kvp_frame_set_slot_nc() routine puts the value (without copying
  *    it) into the frame, associating it with a copy of 'key'.  This
@@ -365,7 +365,7 @@ void          kvp_frame_set_slot_nc(KvpFrame * frame,
  *     at this location, if any, is destroyed.
  */
 void          kvp_frame_set_slot_path (KvpFrame *frame,
-                                       const KvpValue *value,
+                                       KvpValue *value,
                                        const gchar *first_key, ...);
 
 /** The kvp_frame_set_slot_path_gslist() routine walks the hierarchy,
@@ -374,7 +374,7 @@ void          kvp_frame_set_slot_path (KvpFrame *frame,
  *     location, if any, is destroyed.
  */
 void          kvp_frame_set_slot_path_gslist (KvpFrame *frame,
-        const KvpValue *value,
+        KvpValue *value,
         GSList *key_path);
 
 /** @} */
diff --git a/src/libqof/qof/test/test-kvp_frame.c b/src/libqof/qof/test/test-kvp_frame.c
index 0ca7a3c..1c59fe5 100644
--- a/src/libqof/qof/test/test-kvp_frame.c
+++ b/src/libqof/qof/test/test-kvp_frame.c
@@ -59,8 +59,15 @@ teardown( Fixture *fixture, gconstpointer pData )
     test_clear_error_list ();
 }
 
+static gchar* glist_to_string( const GList *list )
+{
+    KvpValue * val = kvp_value_new_glist( list );
+    gchar * ret = kvp_value_to_string( val );
+    kvp_value_delete( val );
+    return ret;
+}
+
 extern KvpFrame* ( *p_get_trailer_make )( KvpFrame *frame, const char *key_path, char **end_key );
-extern gchar* ( *p_kvp_value_glist_to_string )( const GList *list );
 extern KvpFrame* ( *p_get_or_make )( KvpFrame *fr, const char * key );
 extern const KvpFrame* ( *p_kvp_frame_get_frame_or_null_slash_trash )( const KvpFrame *frame, char *key_path );
 extern const KvpFrame* ( *p_get_trailer_or_null )( const KvpFrame * frame, const char * key_path, char **end_key );
@@ -73,9 +80,8 @@ setup_static( Fixture *fixture, gconstpointer pData )
     fixture->frame = kvp_frame_new();
     fixture->hdlrs = NULL;
     init_static_test_pointers();
-    g_assert( p_get_trailer_make && p_kvp_value_glist_to_string &&
-              p_get_or_make && p_kvp_frame_get_frame_or_null_slash_trash &&
-              p_get_trailer_or_null );
+    g_assert( p_get_trailer_make && p_get_or_make && p_get_trailer_or_null && 
+              p_kvp_frame_get_frame_or_null_slash_trash );
 }
 
 static void
@@ -85,7 +91,6 @@ teardown_static( Fixture *fixture, gconstpointer pData )
     g_slist_free_full (fixture->hdlrs, test_free_log_handler);
     test_clear_error_list ();
     p_get_trailer_make = NULL;
-    p_kvp_value_glist_to_string = NULL;
     p_get_or_make = NULL;
     p_kvp_frame_get_frame_or_null_slash_trash = NULL;
     p_get_trailer_or_null = NULL;
@@ -842,7 +847,7 @@ test_kvp_value_compare( void )
     g_test_message( "Test different kvpvalues of all the types" );
     g_assert_cmpint( kvp_value_compare( gint64_orig_value, gint64_copy_value ), == , -1 );
     g_assert_cmpint( kvp_value_compare( gint64_copy_value, gint64_orig_value ), == , 1 );
-    g_assert_cmpint( kvp_value_compare( double_orig_value, double_copy_value ), == , double_compare( 3.3, 3.5 ) );
+    g_assert_cmpint( kvp_value_compare( double_orig_value, double_copy_value ), == , -1 );
     g_assert_cmpint( kvp_value_compare( numeric_orig_value, numeric_copy_value ), == , gnc_numeric_compare( gnc_numeric_orig, gnc_numeric_copy ) );
     g_assert_cmpint( kvp_value_compare( string_orig_value, string_copy_value ), == , strcmp( "abcdefghijklmnop", "abcdefghijklmnop" ) );
     g_assert_cmpint( kvp_value_compare( guid_orig_value, guid_copy_value ), == , guid_compare( guid_orig, guid_copy ) );
@@ -1346,8 +1351,8 @@ test_kvp_value_glist_to_string( Fixture *fixture, gconstpointer pData )
     frame_orig = kvp_frame_new();
 
     g_test_message( "Test empty list" );
-    result = p_kvp_value_glist_to_string( value_list );
-    g_assert_cmpstr( result, == , "[  ]" );
+    result = glist_to_string( value_list );
+    g_assert_cmpstr( result, == , "" );
     g_free( result );
 
     g_test_message( "Test list with simple and complex values" );
@@ -1359,9 +1364,9 @@ test_kvp_value_glist_to_string( Fixture *fixture, gconstpointer pData )
     value_list = g_list_append( value_list, kvp_value_new_frame( frame_orig ) );
     g_assert( value_list );
     g_assert_cmpint( g_list_length( value_list ), == , 6 );
-    result = p_kvp_value_glist_to_string( value_list );
+    result = glist_to_string( value_list );
 
-    g_assert_cmpstr( result, == , "[  KVP_VALUE_GINT64(2), KVP_VALUE_DOUBLE(3.3), KVP_VALUE_NUMERIC(0/1), KVP_VALUE_STRING(abcdefghijklmnop), KVP_VALUE_GLIST([  KVP_VALUE_STRING(abcdefghijklmnop), ]), KVP_VALUE_FRAME({\n}\n), ]" );
+    g_assert_cmpstr( result, == , "KVP_VALUE_GLIST([  KVP_VALUE_GINT64(2), KVP_VALUE_DOUBLE(3.3), KVP_VALUE_NUMERIC(0/1), KVP_VALUE_STRING(abcdefghijklmnop), KVP_VALUE_GLIST([  KVP_VALUE_STRING(abcdefghijklmnop), ]), KVP_VALUE_FRAME({\n}\n), ])" );
     g_free( result );
 
     kvp_glist_delete( value_list );



Summary of changes:
 src/engine/engine.i                  |   2 +-
 src/libqof/qof/Makefile.am           |   3 +-
 src/libqof/qof/kvp-util.cpp          |   2 +-
 src/libqof/qof/kvp-value.cpp         | 322 ++++++++++++++++++++
 src/libqof/qof/kvp-value.hpp         | 127 ++++++++
 src/libqof/qof/kvp_frame-p.hpp       |  14 +
 src/libqof/qof/kvp_frame.cpp         | 561 +++++++----------------------------
 src/libqof/qof/kvp_frame.h           |   8 +-
 src/libqof/qof/test/test-kvp_frame.c |  25 +-
 9 files changed, 591 insertions(+), 473 deletions(-)
 create mode 100644 src/libqof/qof/kvp-value.cpp
 create mode 100644 src/libqof/qof/kvp-value.hpp
 create mode 100644 src/libqof/qof/kvp_frame-p.hpp



More information about the gnucash-changes mailing list