1

I'm using a pattern of assigning a base class to a templated class so that I can put different types in a vector, vis-a-vis Attribute<String> and Attribute<int>, and the reason for this is that I want a vector containing different objects that inherit the same base object.

The problem that I am getting of spurious text being generated relates to the output that is generated once the Base object is retrieved from the vector and cast back to the original Attribute template object.

Problem Output, using inline comments to show where output differed from expectation:


        T (String)
        ID: Id-1
        Key: -�'��,�'�8���Id-1  // expected key1
        Value:                  // expected one

        T (String)
        ID: Id-2
        Key: -�'��,�'�8���Id-2  // expected key2
        Value:                  // expected two

        T (String)
        ID: Id-3
        Key: -�'��,�'�8���Id-3  // expected key3
        Value:                  // expected three

        T (int)
        ID: Id-4
        Key: -�'��,�'�8���Id-4  // expected key4
        Value: 0                // expected 4

        T (String)
        ID: Id-5
        Key: -�'�-�'�8���Id-5   // expected key5
        Value:                  // expected 5

        T (int)
        ID: Id-6
        Key: -�'�0-�'�8���Id-6  // expected key6
        Value: 0                // expected 6

Here is the reproducible example, I've added a Makefile which uses the c++ compiler instead of g++ compiler as on Mac (where I am doing this) C++17 isn't fully implemented yet.

harness.cpp

#include <iostream>
#include "Attribute.h"
#include <vector>

using namespace std;
using String = std::string;

int main()
{

    // TEST THE Attribute CLASS BY ITSELF

    Attribute <String> att("testkey","testvalue", TypeRef::String, "testId");

    cout << "Key: "+att.getKey() << endl;;
    cout << "Value: "+att.getValue() << endl;
    cout << "Id: "+att.getId() << endl;
    cout << endl;

    /* Output:

        Key: testkey
        Value: testvalue
        Id: testId

    */

    // TEST SIX INSTANCES OF Attribute CLASS BEFORE ADDING TO vector

    std::vector<AttributeObject> vector; 

    Attribute<String> q("key1","one",TypeRef::String, "Id-1"); AttributeObject &qBase = q;
    cout << "T (String)" << endl;
    cout << "Id1: " << q.getId() << endl;    
    cout << "Key1: " << q.getKey() << endl;
    cout << "Value1: " << q.getValue() << endl;

    cout << endl;

    Attribute<String> w("key2","two",TypeRef::String, "Id-2"); AttributeObject &wBase = w; 
    cout << "T (String)" << endl;
    cout << "Id2: " << w.getId() << endl;    
    cout << "Key2: " << w.getKey() << endl;
    cout << "Value2: " << w.getValue() << endl;

    cout << endl;

    Attribute<String> e("key3","three",TypeRef::String, "Id-3"); AttributeObject &eBase = e;
    cout << "T (String)" << endl;
    cout << "Id3: " << e.getId() << endl;    
    cout << "Key3: " << e.getKey() << endl;
    cout << "Value3: " << e.getValue() << endl;

    cout << endl;

    Attribute<int> r("key4",4,TypeRef::Int, "Id-4"); AttributeObject &rBase = r; 
    cout << "T (int)" << endl;
    cout << "Id4: " << r.getId() << endl;    
    cout << "Key4: " << r.getKey() << endl;
    cout << "Value4: " << r.getValue() << endl;

    cout << endl;

    Attribute<int> t("key5",5,TypeRef::String, "Id-5"); AttributeObject &tBase = t; 
    cout << "T (int)" << endl;
    cout << "Id5: " << t.getId() << endl;    
    cout << "Key5: " << t.getKey() << endl;
    cout << "Value5: " << t.getValue() << endl;

    cout << endl;

    Attribute<int> y("key6",6,TypeRef::Int, "Id-6"); AttributeObject &yBase = y; 
    cout << "T (int)" << endl;
    cout << "Id6: " << y.getId() << endl;    
    cout << "Key6: " << y.getKey() << endl;
    cout << "Value6: " << y.getValue() << endl;

    cout << endl;

    cout << endl;

    /* Output:

        T (String)
        Id1: Id-1
        Key1: key1
        Value1: one

        T (String)
        Id2: Id-2
        Key2: key2
        Value2: two

        T (String)
        Id3: Id-3
        Key3: key3
        Value3: three

        T (int)
        Id4: Id-4
        Key4: key4
        Value4: 4

        T (int)
        Id5: Id-5
        Key5: key5
        Value5: 5

        T (int)
        Id6: Id-6
        Key6: key6
        Value6: 6

    */

    vector.push_back(qBase);
    vector.push_back(wBase);
    vector.push_back(eBase);
    vector.push_back(rBase);
    vector.push_back(tBase);
    vector.push_back(yBase);

    // TEST ALL Attribute CLASS INSTANCES AS EXTRACTED FROM A vector

    int x = 0;
    for (AttributeObject baseObject : vector) {

        TypeRef typeRef = baseObject.getTypeRef();
        if(typeRef == TypeRef::String)
        {
            cout << endl;
            cout << "T (String)" << endl;
            Attribute <String> *pChild =  (Attribute <String> *) &baseObject;
            cout << "ID: " << pChild->getId() << endl; 
            const String sKey = pChild->getKey();
            cout << "Key: " << sKey << endl;
            const String sValue = pChild->getValue();
            cout << "Value: " << sValue << endl;
        }
        else if(typeRef == TypeRef::Int)
        {
            cout << endl;
            cout << "T (int)" << endl;
            Attribute <int> *pChild =  (Attribute <int> *) &baseObject;
            cout << "ID: " << pChild->getId() << endl; 
            const String sKey = pChild->getKey();
            cout << "Key: " << sKey << endl;
            const int iValue = pChild->getValue();
            cout << "Value: " << (int)iValue << endl;

        }
        x++;
    }

    /* Output (with differing expected values added as inline comments)

        T (String)
        ID: Id-1
        Key: -�'��,�'�8���Id-1  // expected key1
        Value:                  // expected one

        T (String)
        ID: Id-2
        Key: -�'��,�'�8���Id-2  // expected key2
        Value:                  // expected two

        T (String)
        ID: Id-3
        Key: -�'��,�'�8���Id-3  // expected key3
        Value:                  // expected three

        T (int)
        ID: Id-4
        Key: -�'��,�'�8���Id-4  // expected key4
        Value: 0                // expected 4

        T (String)
        ID: Id-5
        Key: -�'�-�'�8���Id-5   // expected key5
        Value:                  // expected 5

        T (int)
        ID: Id-6
        Key: -�'�0-�'�8���Id-6  // expected key6
        Value: 0                // expected 6
    */

    return 0;
}

Attribute.cpp (here just for the sake of the Makefile because the c++ complier generates a nasty warning if you don't use a .cpp file):

#include "Attribute.h"

Attribute.h:

#include <iostream>
#include <string>
#include <type_traits>
#include <vector>

using String = std::string;

enum class TypeRef {   
    String,
    Int
};

class AttributeObject{
    public:
            AttributeObject() {}
            AttributeObject(TypeRef typeRef, String Id) : typeRef(typeRef), id(Id) {}

            TypeRef getTypeRef()
            {
                return this->typeRef;
            }

            String getId()
            {
                return this->id;
            }


    protected:
            TypeRef typeRef;
            String id;
};

template<class T>
class Attribute : public AttributeObject {
public:
    using value_type = T;

    Attribute(const String& Key, const T& Value, const TypeRef& TypeRef, const String& Id) : 
        AttributeObject(TypeRef, Id),
        key(Key),        
        value(Value)
        {}

    String const& getKey() const {
        return key;
    };
    T const& getValue() const {
        return value;
    }

    TypeRef const& getTypeRef() const {
        return typeRef;
    }

private:
    String key;
    T value;
};

Makefile:

CC=c++
FLAGS=-c -g -std=c++17

All: build

mkdirs:
    # In mkdirs:
    mkdir -p obj

build: clean mkdirs harness.o Attribute.o
    # In build:
    $(CC) obj/harness.o obj/Attribute.o -o harness
    ls

harness.o: harness.cpp
    # harness.o:
    $(CC) $(FLAGS) harness.cpp -o obj/harness.o
    ls obj

Attribute.o: Attribute.cpp
    $(CC) $(FLAGS) Attribute.cpp -o obj/Attribute.o
    ls obj

clean:
    # In clean:
    rm -rf obj
    ls

Kind regards.

Xamtastic
  • 105
  • 1
  • 9
  • You are encountering [object slicing](https://stackoverflow.com/q/274626). See [the answers of this question](https://stackoverflow.com/q/52222665) for a list of possible solutions for your problem. – Mike Oct 04 '19 at 11:29
  • To add to the above, the problem is that your vector stores objects of type AttributeObject, so the information expanded in object Attribute is lost. A solution would be to store pointers to AttributeObject and then dynamic_cast them down to Attribute. Of course, then you would have to take into account the life span of your objects, in order to keep them alive while you operate with the pointers. – mcabreb Oct 04 '19 at 14:34
  • Other solutions could range to using std::variant or std::any, if they suit your needs. – mcabreb Oct 04 '19 at 14:37
  • Thanks @mcabreb, I was fortunate enough that Ted (below) rewrote the implementation for me, just on the lifespan comment, these are very transient objects as I'm just building the vectors so that I can use them to generate JSON on the fly at the point that that the JSON text needs to be thrown into a Bluetooth terminal. Thanks for your help, I'm a career C# chappie converting to C++. – Xamtastic Oct 08 '19 at 17:45

1 Answers1

0

As mentioned in the comments, the biggest problem in this code is object slicing and to work around that you should use base class pointers or references. In a vector you can store pointers but not real references (you can use std::reference_wrapper though).

You have to decide if the vector should own the objects or if it should only keep pointers to objects whos lifespan is controlled separately from the vector.

std::vector<BaseClass*> v1;           // objects will live on even when the vector is destroyed
std::vector<std::unique_ptr<BaseClass>> v2; // objects are destroyed if the vector is destroyed

In your test code, you've used the first option so I'll go with that, but it's easy (and often preferable) to change that.

Here's an idea of how to make the changes needed. I hope the comments in the code explains most of it.

Attribute.h

// add a header guard to not accidentally include it into the same translation unit more than once
#ifndef ATTRIBUTE_H
#define ATTRIBUTE_H

#include <iostream>
#include <string>
#include <typeinfo> // typeid()

using String = std::string;

// An abstract base class for all Attribute<T>'s
// Since "key" is common for them all, I've put it in here.
class AttributeBase {
public:
    AttributeBase(const String& k) : key(k) {}
    virtual ~AttributeBase() = 0; // pure virtual

    String const& getKey() const {
        return key;
    };

    // all descendants must implement a print method
    virtual std::ostream& print(std::ostream&) const = 0;

    // trust all Attribute<T>'s to get direct access to private members
    template<typename T>
    friend class Attribute;
private:
    String key;
};

// AttributeBase is an abstract base class but with a default
// destructor to not force descendants to have to implement it.
AttributeBase::~AttributeBase() {}

// streaming out any AttributeBase descendant will, via this method, call the virtual
// print() method that descendants must override 
std::ostream& operator<<(std::ostream& os, const AttributeBase& ab) {
    return ab.print(os);
}

template<class T>
class Attribute : public AttributeBase {
public:
    using value_type = T;

    Attribute(const String& Key, const T& Value) :
        AttributeBase(Key),
        value(Value)
    {}

    T const& getValue() const {
        return value;
    }

    std::ostream& print(std::ostream& os) const override {
        // Print an implementation defined name for the type using typeid()
        // and then "key" and "value".
        // Direct access to "key" works because of the "friend"
        // declaration in AttributeBase. We could have used getKey()
        // though, but this shows one use of "friend".
        return
            os << "type:  " << typeid(value).name() << "\n"
               << "key:   " << key << "\n"
               << "value: " << value << "\n";
    }

private:
    T value;
};

// end of header guard
#endif

harness.cpp

// include your own headers first to catch include chain errors more easily
#include "Attribute.h"

#include <iostream>
#include <vector>
#include <memory>

// using namespace std; // bad practice:
// https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice

using String = std::string;

int main()
{
    // TEST THE Attribute CLASS BY ITSELF
    // in the following functions we're using the added operator<< to let the objects
    // print their own values

    Attribute <String> att("testkey","testvalue");
    std::cout << "-- att --\n" << att << "\n";

    // TEST SIX INSTANCES OF Attribute CLASS BEFORE ADDING TO attvec

    // use base class pointers to avoid slicing
    std::vector<AttributeBase*> attvec;

    Attribute<String> q("key1","one");
    std::cout << "-- q ---\n" << q << "\n";

    Attribute<String> w("key2","two");
    std::cout << "-- w ---\n" << w << "\n";

    Attribute<String> e("key3","three");
    std::cout << "-- e --\n" << e << "\n";

    Attribute<int> r("key4",4);
    std::cout << "-- r --\n" << r << "\n";

    Attribute<int> t("key5",5);
    std::cout << "-- t --\n" << t << "\n";

    Attribute<int> y("key6",6);
    std::cout << "-- y --\n" << y << "\n";

    // added a 7:th object with a different type

    Attribute<double> u("key7", 7.12345);
    std::cout << "-- u --\n" << u << "\n";

    // put pointers to the objects in the vector
    attvec.push_back(&q);
    attvec.push_back(&w);
    attvec.push_back(&e);
    attvec.push_back(&r);
    attvec.push_back(&t);
    attvec.push_back(&y);
    attvec.push_back(&u);

    // TEST ALL Attribute CLASS INSTANCES AS EXTRACTED FROM A vector
    std::cout << "--\n";

    for (AttributeBase const* baseObject : attvec) {

        // try to dynamic_cast to the types for which you have special handling
        // if( <init> ; <condition> ) { ...

        if(auto pChild = dynamic_cast<Attribute<String> const*>(baseObject); pChild)
        {
            std::cout << "T (String)\n";
            const String sKey = pChild->getKey();
            std::cout << "Key: " << sKey << "\n";
            const String sValue = pChild->getValue();
            std::cout << "Value: " << sValue << "\n";
            // or let the user defined streaming operator for the type do the work:
            std::cout << *pChild << "\n\n";
        }
        else if(auto pChild = dynamic_cast<Attribute<int> const*>(baseObject); pChild)
        {
            std::cout << "T (int)\n";
            const String sKey = pChild->getKey();
            std::cout << "Key: " << sKey << "\n";
            const int iValue = pChild->getValue();
            std::cout << "Value: " << iValue << "\n";
            // or let the user defined streaming operator for the type do the work:
            std::cout << *pChild << "\n\n";
        } else {
            std::cout << "T (generic)\n";
            const String sKey = baseObject->getKey();
            std::cout << "Key: " << sKey << "\n";
            /* the getValue() method does not exist in the base class
            auto genValue = baseObject->getValue();
            cout << "Value: " << genValue << "\n";
            */
            // or let the user defined streaming operator for the type do the work:
            std::cout << *baseObject << "\n";
        }
    }
}

I removed the dependency to Attributes.cpp in the makefile so you can remove that file. I also added some things that can come in handy when chasing bugs and made a generic rule for mapping <file>.cpp to obj/<file>.o. I use gmake so it may contain gmake specific things that makes it fail on your side. Just disregard it in that case. Some of the options may still be useful.

Makefile

CC=c++

MINIMAL_WARNINGS=-Wall -Wextra -pedantic

BONUS_WARNINGS=-Werror -Wshadow -Weffc++ -Wconversion -Wsign-conversion -Woverloaded-virtual \
               -Wold-style-cast -Wwrite-strings -Wcast-qual -Wnoexcept -Wnoexcept-type \
               -Wpessimizing-move -Wredundant-move -Wstrict-null-sentinel -Wunreachable-code \
               -Wnull-dereference -Wsequence-point -pedantic-errors

# scan-build — Clang static analyzer
STATIC_ANALYSIS = scan-build -v --force-analyze-debug-code

# SANITIZER options using libasan.
# libasan - good for catching and displaying misc errors in runtime instead of just resulting
#           in a "Segmentation fault (core dumped)".
SANITIZER=-fsanitize=undefined -fsanitize=address

# turn on the bonus warnings if you'd like to fix misc things that are usually good to fix.
#WARNINGS=$(MINIMAL_WARNINGS) $(BONUS_WARNINGS)
WARNINGS=$(MINIMAL_WARNINGS)

FLAGS=-g3 -std=c++17 $(WARNINGS)

# collect all your .cpp files - remember to remove Attribute.cpp
SRC=$(wildcard *.cpp)

# Create a list of object files needed before linking.
# For each "%.cpp" file in SRC, "obj/%.o" will be put in OBJS.
OBJS=$(patsubst %.cpp,obj/%.o,$(SRC))

TARGETS=harness

All: $(TARGETS)

harness: $(OBJS)
        @# turn on SANITIZER on if you have libasan installed (linking will fail if you dont)
        @#$(CC) $(FLAGS) $(SANITIZER) -o harness $(OBJS)
        $(CC) $(FLAGS) -o harness $(OBJS)

# A generic object file rule. It requires a .cpp file and that the obj directory exists.
obj/%.o : %.cpp obj Attribute.h
        @# turn on STATIC_ANALYSIS if you have scan-build installed
        @#$(STATIC_ANALYSIS) $(CC) $(FLAGS) -c -o $@ $<
        $(CC) $(FLAGS) -c -o $@ $<

# The object directory target
obj:
        mkdir -p obj

clean:
        rm -rf obj $(TARGETS)
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • 1
    Ted, this is simply fabulous on so many levels, I'm going to need a day to assimilate this and then a day to implement it in my project, thank you so much, I'll follow up with a proper reflective comment sometime next week, I've scheduled Friday and Monday for this, maybe earlier, depending on what tickets I finish this week! Thanks again for all the work! Needless to say, I marked it as the answer! – Xamtastic Oct 08 '19 at 17:35
  • @Xamtastic Many thanks for your kind words! Also check out `std::variant` and `std::unordered_map` etc. that may help you to come up with a completely different solution :-) – Ted Lyngmo Oct 08 '19 at 17:39