Avatar of Rothbard
RothbardFlag for United Kingdom of Great Britain and Northern Ireland

asked on 

Conversion operator causing compilation error

I have implemented a string-like class, called Str, and want to add a conversion to bool type, in order to use a Str object as a condition. The Str class is quite simple and relies on the STL vector class. Everything works fine, except that now, if I try for example to do the following:

Str greeting = "Hello, " + name + "!";

Open in new window

where "name" is a Str, then I get a compilation error, saying "'operator +': 2 overloads have similar conversions, could be 'Str operator +(const Str &,const Str &)' or 'built-in C++ operator+(const char [8], int)'.

I understand the reason for the error, but I am not sure how to fix it. How can I get around this? Below I have copied a minimal working example.

#include <iterator>
#include <vector>
#include<iostream>

using namespace std;

class Str
{

private:
    std::vector<char> data;

public:
    typedef std::vector<char>::size_type size_type;
    typedef std::vector<char>::iterator iterator;
    typedef std::vector<char>::const_iterator const_iterator;

    iterator begin() { return data.begin(); }
    const_iterator begin() const { return data.begin(); }
    iterator end() { return data.end(); }
    const_iterator end() const { return data.end(); }


    // default constructor must be defined explicitly, since non-default constructors are also defined
    Str() { }
    Str(const size_type n, const char c) : data(n, c) {  }
    Str(const char* cp)
    {
        std::copy(cp, cp + std::strlen(cp), std::back_inserter(data));
    }
    // Since this is a template, the iterators can be anything: pointers in an array of chars, std::vector<string> iterators, std::std::vectortor<string> iterators, std::list<string> iterators, etc.
    template <class In> Str(In b, In e)
    {
        std::copy(b, e, std::back_inserter(data));
    }
    char& operator[](size_type i) { return data[i]; }
    const char& operator[](size_type i) const { return data[i]; }
    Str& operator+=(const Str& s)
    {
        std::copy(s.data.begin(), s.data.end(), std::back_inserter(data));
        return *this;
    }
    size_type size() const
    {
        return data.size();
    }

    // conversion operators
    operator std::vector<char>() const;
    operator bool() const;

    template <class In> void insert(iterator dest, In b, In e)
    {
        data.insert(dest, b, e);
    }

};

Str operator+(const Str& s1, const Str& s2)
{
    Str s = s1;
    s += s2;
    return s;
}

Str::operator std::vector<char>() const
{
    std::vector<char> ret = data;
    return ret;

}

Str::operator bool() const
{
    if (data.size() > 0)
        return true;
    return false;
}


int main()

{

    Str name = "Joe";
    Str greeting = "Hello, " + name + "!";

    return 0;
}

Open in new window

C++

Avatar of undefined
Last Comment
Fabrice Lambert
Avatar of phoffric
phoffric

Here is one work-around:
Str greeting = Str("Hello, ") + name + Str("!");

Open in new window

SOLUTION
Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

Blurred text
THIS SOLUTION IS ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
See Pricing Options
Start Free Trial
ASKER CERTIFIED SOLUTION
Avatar of sarabande
sarabande
Flag of Luxembourg image

Blurred text
THIS SOLUTION IS ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
Avatar of sarabande
sarabande
Flag of Luxembourg image

sorry evilrix, i did not refresh  before posting.

Sara
Side note:
Remove this horrible "using namespace std" directive, it has no business in modern C++ code.

You should take adventage of the std::string class, instead of C-Style strings.

You can use the built-in constructor instead of writing a default one that does nothing:
str() = default;

Open in new window


Instead of re-inventing the wheel, what about inheriting privatly from std::vector<char> ?
You can expose the functionalities you want with the using directive.
Sample code:
class str: private std::vector<char>
{
public:
        // bring iterators to visible scope
    using std::vector<char>::begin;
    using std::vector<char>::end;
        // constructor and other class stuffs
    str() = default;
    str(std::string const&);
    str(char);
        // ect ...
private:
};

Open in new window

Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

>> Remove this horrible "using namespace std" directive, it has no business in modern C++ code.
Why? It's a matter of personal preference. There is nothing inherently wrong with importing a namespace as long as you understand what it does.

>> You should take adventage of the std::string
I've already made that point.

>> Instead of re-inventing the wheel, what about inheriting privatly from std::vector<char>
This is bad advice. The std container libraries are not designed to be inherited and doing so can result in undefined behaviour if used polymorphically. There are only a handful of std objects that are safe to inherit, std::exception being one of them, the various iterator helper classes being another example.

As I've already said, without knowing the objectives of what the OP is trying to achieve, we can't really give any advice beyond explaining how to fix the original compiler error, which has already been covered - both by myself and some additional information by sarabande.

>> sorry evilrix, i did not refresh  before posting.
No worries, Sara. It happens :)
Avatar of sarabande
sarabande
Flag of Luxembourg image

inheriting privatly from std::vector<char>

inheriting from STL objects is not recommended because the classes don't have a virtual destructor.

also a std::vector<char> is a container (or buffer) but commonly IS not A string.

Sara
>>>> Remove this horrible "using namespace std" directive, it has no business in modern C++ code.

>>Why? It's a matter of personal preference. There is nothing inherently wrong with importing a namespace as long as you understand what it does.
It is considered a bad practice:
It pollute the global namespace.
Conflicts can emerge when you use multiple libraries.
It is against the C++ philosophy: Don't pay for what you don't use.
It is the less worst solution to ensure compatibility with very old code base. Is your code very old ? I doubt so.
The using directive is available if you want to save typing.
More on the this article:
https://www.geeksforgeeks.org/using-namespace-std-considered-bad-practice/
Consider the following code:
namespace MyNameSpace
{
    class string
    {
    };
}

using namespace std;    // ops !
using namespace MyNameSpace;    // ops !

int main()
{
    string myString;    // angry compiler !!
}

Open in new window


>>>> Instead of re-inventing the wheel, what about inheriting privatly from std::vector<char>

>>This is bad advice.
Private inheritance !!
With private inheritance, a class is implemented "as a" (while a class "is a" with public inheritance).
There is no polymorphism involved here.
Avatar of sarabande
sarabande
Flag of Luxembourg image

With private inheritance, a class is implemented "as a"

in my opinion "as a" includes the "is a". at least it is not conflicting. the contrary to inheritance is the association (or "has a" relation) with two flavors: 'composition' ("the B is a part of A") and 'aggregation' ("A contains a B").

private inheritance means you have a class where you want to conceal the (details of) implementation by putting the implementation code into a baseclass (where you don't have to provide any source code beside of the interface of your derived class). if you derive class Str privately from std::vector which is an array class (and never more than an array class), then the derived class would also be an array class what is a viewpoint that ignores the specialities of a string class. also Str would implement all its string functionality itself what shows clearly that private derivation is a wrong design for this. moreover, consider the point evilrix has made that STL classes are not designed for derivation as they don't have virtual functions.

Str can "use an" array class as data storage what means the string class "has an" array (i would say it is composed of an array and optional size parameters)  and would not derive from an array class.

Sara
Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

>> It is considered a bad practice:
By whom? Refer me to an authorative source that says this

>> It pollute the global namespace.
It doesn't pollute the global namespace. It imports those symbols into the current namespace (which may or may not be global) within the current translation unit. It IS bad practice to us using namespace in headers, that will pollute the global namespace, but importing into the current namespace in the current translation unit does not, otherwise you could argue that about using any namespace, not just the std one. Conventionally, people tend to not do this (I don't), but there is nothing inherently wrong with it in terms of good or bad practice.

>> Conflicts can emerge when you use multiple libraries.
As they can when you import any symbol into the current namespace, this is not specific to std namespace.

>> It is against the C++ philosophy: Don't pay for what you don't use.
To a certain extent, I will agree here, it is better to only import the symbols you need, so if you need std::string then only import std::string; however, again, this is NOT specific to the std namespace and your comment was regarding that.

>> It is the less worst solution to ensure compatibility with very old code base
I'm afraid I do not understand your point.

>> The using directive is available if you want to save typing.
Actually, it is way more useful than just saving on typing. It also lets you switch between different implementations of the same thing at compile time. As an example, you could have a namespace for Linux and one for win32 and your code can import either or the other, depending upon a precompile directive.

Anyway, if you want to save typing you can always use namespace aliases.

>> More on the this article:
Well, firstly I don't consider geeksforgeeks to be authoritative. Secondly, the example they give is pretty contrived. Thirdly - and I say this again - the same is true of using any namespace, this is not specific to the std namespace.

The point I am making is that there is nothing specifically bad about using the std namespace as there is (or isn't) with any namespace. The std namespace is NOT a special case. I'm not arguing for or against doing it, I'm just saying there is nothing specifically bad practice about importing std that doesn't apply to any other namespace and whether you choose to import a namespace or not is really down to you...

...unlike subclassing STL containers, which is just BAD!

>> With private inheritance, a class is implemented "as a" (while a class "is a" with public inheritance).

The problem is that it only takes one joker to do something like the following and you have undefined behaviour on your hands and an incredibly hard to locate defect...

#include <iostream>
#include <memory>

class foo
{
public:
	~foo()
	{
		std::cout << "foo destructor" << std::endl;
	}
	
	void hello()
	{
		std::cout << "hello" << std::endl;
	}
};

class bar : private foo
{
public:
	virtual ~bar()
	{
		// never run
		std::cout << "bar destructor" << std::endl;
	}
};

void need_foo(std::unique_ptr<foo> p)
{
	p->hello();
}

int main()
{
	auto && p = std::unique_ptr<foo>((foo *) new bar);
	need_foo(std::move(p));
}

Open in new window


There's just no real valid reason to inherit from an STL container, privately or otherwise. In the example we're looking at here, you're suggesting private inheritence just to save typing (ironically, unlike using a namespace; however, ask youself if this is really a valid reason for usingn inheritence. You still have to have a "using" statement to explose the private functions publically. In the process, you're abusing the inheritence system, running the risk of undefined behaviour (see above) and really not saving on much typing.

With the advent of perfect forwarding in C++14, the correct (and safer) approach is to have the vector as a member and just expose the methods you need to using forwarding functions. You end up with code that isn't much more verbose, is self documenting, the relationships of the objects are clear and you don't run the risk of you accidentally casting to the private base class at some point and then deleting via the base class.

If you truly want to do something like this (as in save yourself typing when exposing STL containers), you're better off using the C++ template system and a policy based framework...

#include <iostream>
#include <vector>

/////////////////////////////////////////////////
// Write once, use again and again

//--->
// Inheritence policy to allow either poly or non-poly
enum class Policy { Polymorphic, NonPolymorphic };

template<Policy>
struct InheritencePolicy;

template <>
struct InheritencePolicy<Policy::Polymorphic>
{
	virtual ~InheritencePolicy() = default;
};

template <>
struct InheritencePolicy<Policy::NonPolymorphic>
{
	~InheritencePolicy() = default;
};

//--->
// Reusable interface - specialise for each STL container you wish to use
template <typename T, Policy P>
class ReusableInterface;

// specialised inteface for vector<T>
template<typename T, Policy P>
class ReusableInterface<std::vector<T>, P> : public InheritencePolicy<P>
{
public:
	auto begin()
	{
		return cont_.begin();
	}

	auto end()
	{
		return cont_.end();
	}

private:
	std::vector<T> cont_;
};

template<typename T, Policy P>
using ReusableInterfaceVector = ReusableInterface<std::vector<T>, P>;

template<typename T>
using ReusableInterfaceVectorPoly = ReusableInterface<
   std::vector<T>, Policy::Polymorphic
>;

template<typename T>
using ReusableInterfaceVectorNonPoly = ReusableInterface<
   std::vector<T>, Policy::NonPolymorphic
>;

// add more specialisations here

/////////////////////////////////////////////////

// client code
class foo : public ReusableInterfaceVectorPoly<int>
{
	public:	
};

int main()
{
	auto && o = foo();
	auto && itr = o.begin();
}

Open in new window


Yes, this looks like a lot of code; however, the point is that you only need to write it once and then you can re-use it
@saraband:
Consider the Following code:
class container: private std::vector<char>
{
};

void foo(std::vector<char> const& in)
{
}

int main()
{
    container myContainer;
    foo(myContainer);
}
It won't compile, because the container class is not an std::vector<char> class, but it is implemented as an std::vector<char> class (the difference is subtile).
The adventage here is you won't have to forward many existing functionalities, just bring them into visible scope and you are done.
Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

>> It won't compile, because...
You're missing the point. The issue is when you try to delete an object by a pointer to its dynamic and not static type. The C++ standard is unusually clear on this, the behaviour is undefined (that's bad!). Unfortunately, it's all too easy to cast away this private inheritance, via a pointer. Should you do this? No? Will you or someone else do it at some point because your private inheritance is stopping you from passing the object to a function that needs a vector? Yes!

STL containers are not designed to be inherited.

#include <vector>

class container: private std::vector<char> {
};

void foo(std::vector<char> const * p)
{
	delete p;
}

int main()
{
    // This cast is perfectly legal and -- sometimes -- even useful :)
    std::vector<char> * p = (std::vector<char> *) new container;
    foo(p);
}

Open in new window

At least provide correct code instead of akward one.
Your sample is exactly like unsafly casting a pointer to integer to a pointer to char: char* ptr = (char*) new int;
C++ allow many things, even shooting yourself in the foot (but that's your responsibility).

From my tests:
#include <iostream>
#include <memory>

class bar
{
public:
	~bar()
	{
		std::cout << "bar destructor" << std::endl;
	}
};

class foo : private bar
{
public:
	 ~foo()
	{
		std::cout << "container destructor" << std::endl;
	}
};

int main()
{
	{
		    // no pointer at all
		foo myFoo;
	}
	{
		    // *sigh* raw pointers
		foo* ptr{ new foo };
		delete ptr;
	}
	{
		    // raw pointers 2
		// bar* ptr = new foo;		// does not compile, classes are not substitutable
		// delete ptr;
	}
	{
			// smart pointer
		std::unique_ptr<foo> ptr{ std::make_unique<foo>() };
	}
	{
                    // smart pointer 2
		// std::unique_ptr<bar> ptr = std::make_unique<container>();    // does not compile, classes are not substitutable
	}
	{
			// smart pointer and move
		std::unique_ptr<foo> ptr{ std::move(std::make_unique<foo>()) };
	}
}

Open in new window

Both destructors are called every time.
Virtual mecanisms arn't playing here.
As for an official source, maybe this link:
https://isocpp.org/wiki/faq/coding-standards#using-namespace-stdhttps://isocpp.org/wiki/faq/coding-standards#using-namespace-std
To me, the key sentence is:
"The fly in that ointment is that it lets the compiler see any std name, even the ones you didn’t think about."


>> It is the less worst solution to ensure compatibility with very old code base
I'm afraid I do not understand your point.
The directive was put in place when namespaces were added to C++, and to prevent the excessive work of digging trough all the existing code to update them to the new standard.
A simple line of code allowed  the old code base to keep working with a minimum of efforts.
That should be its sole purpose.
Avatar of Rothbard
Rothbard
Flag of United Kingdom of Great Britain and Northern Ireland image

ASKER

Thanks to all who replied! By the way, this was just an exercise in a book I have been using to brush up my C++ ("Accelerated C++" by Koenig and Moo, ch. 12). In practice, of course I would not implement my own string class.
Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

>> Your sample is exactly like unsafly casting
It is exactly like unsafe casting; however, it's also perfectly legal (in fact, the C++ standard suggests this as a way of accessing members of a base class that has been inherited privately). The point is, at some point someone will do it and you'll end up (potentially) with a hard to locate defect.

>> C++ allow many things, even shooting yourself in the foot
Indeed it does... for example, subclassing STL containers! The point is, if you know what you're doing and you really want to take the risk then go for it; however, it is NOT something we should be recommended to others... especially others who are learning C++. At the very least, if you do recommend something like that be sure to explain the caveats that go with it.

>> Both destructors are called every time.
Well of course, you're deleting by a pointer to the static and not dynamic type. The problem is if you delete via a pointer to the dynamic type and there is no virtual destructor (either explcit or implicit) in the dynamic class type.

>> The fly in that ointment is that it lets the compiler see any std name
And, as I've already explained... this is true of any namespace you import. The std namespace is NOT a special case. There is no rule, writen or unwritten, that says it's bad practice beyond that of importing any namespace.

Personally, I don't import the std namespace; however, I wouldn't berate someone for doing so. Just as an aside, go to https://ideone.com/ and create a new C++ project. Look at the 2nd line. I'm not suggesting this is evidence that it's okay to do this, I just think it's amusing that ideone does this by default (personally, I wouldn't - heh). Apart from that, it's a really cool website to hack sample code on :)

>> The directive was put in place when namespaces were added to C++, and to prevent the excessive work
If you mean "using namespace", as I've already explained to you - it has more uses that just saving typing or preventing excessive work.

Anyway, I think we've discuss this to death now :)
Thanks for the discussion.

>> Thanks to all who replied
You're very welcome. Good luck with your brushing up and if you have any questions about what's been discussed above, don't hesitate to post back here. I'm sure anyone of us will be only too happy to clarify.

All the best, very one.
>>>> Both destructors are called every time.
>>Well of course, you're deleting by a pointer to the static and not dynamic type. The problem is if you delete via a pointer to the dynamic type and there is no virtual destructor (either explcit or implicit) in the dynamic class type.
Again no point in having a pointer(smart or not) to the base class, since with private inheritance, the derived class is not a base class, thus no dynamic types are involved.
So initialising a base pointer to the derived class is a no sens to begin with.

If you look at my last code snipet, 2 samples are commented out because they do not compile (attempt to use raw and smart pointers to the base class).

The point is, at some point someone will do it and you'll end up (potentially) with a hard to locate defect.
Careless codes are not my reponsibility.
C++
C++

C++ is an intermediate-level general-purpose programming language, not to be confused with C or C#. It was developed as a set of extensions to the C programming language to improve type-safety and add support for automatic resource management, object-orientation, generic programming, and exception handling, among other features.

58K
Questions
--
Followers
--
Top Experts
Get a personalized solution from industry experts
Ask the experts
Read over 600 more reviews

TRUSTED BY

IBM logoIntel logoMicrosoft logoUbisoft logoSAP logo
Qualcomm logoCitrix Systems logoWorkday logoErnst & Young logo
High performer badgeUsers love us badge
LinkedIn logoFacebook logoX logoInstagram logoTikTok logoYouTube logo