Wrong constructor is being called

Question

I have a class which creates different kinds of variables.

And through some macro defines we want to initialize those variables.

#define NUMBER(number)      JsonVariable(number)
#define STRING(text)        JsonVariable(text)
#define TRUE                JsonVariable(true)
#define FALSE               JsonVariable(false)

Numbers are initialized fine, but Strings call the Bool constructor for unknown reasons.

JsonVariable(string x)
    : type(String), s(x)
{
    cout << "String" << x << endl;
    sleep
}
JsonVariable(bool x)
    : type(Boolean), b(x)
{
    cout << "Boolean" << x << endl;
    sleep
}

If I comment out the Bool constructor then the String one is called.

Any suggestions?

EDIT: This is a string constructor with the defined macros. std::string is used in the constructor.

JSON(test) = STRING("Hello")

Type is a defined enum. Also macros must be used as part of this assignment.

EDIT2: For clarification. Here's the enum type.

std::string is used with namespace std and thus single string. Also String is from the enum type so

String != string

typedef enum {
    Null, Integer, Double, Boolean, String, Object, Array
} datatype;

Do you call it with a real string or `const char*` (like `"myString"`)? — lorro, Jan 14 '17 at 19:08
Does this `JsonVariable(string x) : type(String), s(x)` compile? — Ed Heal, Jan 14 '17 at 19:10
Good luck with the guess - considering the poster is using `string` as a parameter — Ed Heal, Jan 14 '17 at 19:15
std::string is used, there is an using namespace std; uptop... also String is an enum type as I have edited my post. — mkanakis, Jan 14 '17 at 19:19
@EdHeal: That's a _member initialiser list_. Here's some further reading for you: http://stackoverflow.com/q/1711990/560648 Hope that helps! — Lightness Races in Orbit, Jan 14 '17 at 19:25
@LightnessRacesinOrbit - Where is the `enum` in the question — Ed Heal, Jan 14 '17 at 19:27
@EdHeal: Not sure whether you're being deliberately obtuse now. Obviously it compiles for the OP, and they said it's an enum. Yes they should have provided an MCVE. But what is your point beyond that? — Lightness Races in Orbit, Jan 14 '17 at 19:28
@LightnessRacesinOrbit - code says in the initialization list ` type(String)` - Where is `type` where is `String`? — Ed Heal, Jan 14 '17 at 19:30
@LightnessRacesinOrbit sleep is a temp solution for keeping the window open, nothing special just a define — mkanakis, Jan 14 '17 at 19:40
Yep! This project we interpret JSON language commands and compile them as C++ . Thus we need JSON object declarations as C++ code. You don't write C++ you write JSON ;) — mkanakis, Jan 14 '17 at 19:44
@Davelis4: You can't do any of that with macros (neither is `sleep` a "JSON object declaration"), so that doesn't really answer my question. — Lightness Races in Orbit, Jan 14 '17 at 19:45
*"we interpret JSON language commands and compile them as C++"* - What is a "JSON command"? If your goal is to read/write JSON files, there are already existing powerful libraries available out there... — Holt, Jan 14 '17 at 19:49
You may want to check out RapidJSON or JSONCpp rather than rolling your own. Be warned that JSONCpp has some surprises of its own (at least in the version I used). — marko, Jan 15 '17 at 10:58
Since these are constructors, declaring them as `explicit` will inhibit implicit conversions, and prevent the surprises you see. You'll probably find your code doesn't compile either until you've provided more overloads, so you may consider this to be disadvantage of using it. — marko, Jan 15 '17 at 11:02

lorro · Answer 1 · 2017-01-14T20:22:38.573

3

You might experience the effect of conversion rules. If you pass a const char*, bool overload is preferred over std::string&:

void f(std::string&) { std::cout << "string"; }
void f(bool) { std::cout << "bool"; }

int main() {
    f("abc"); // prints 'bool'
}

This is logical: pointers are usually checked for being non-zero, thus conversion to int / bool types is desirable. On the other hand, binding to a const std::string& means a std::string constructor and taking reference. Binding to std::string& is impossible here as temporaries don't bind to non-const references.

Workaround is either to manually create a string or, preferably, to have a constructor for const char*.

edited Jan 14 '17 at 20:22

answered Jan 14 '17 at 19:13

lorro

10,687
23
36

There is no casting here. – Lightness Races in Orbit Jan 14 '17 at 19:13
@LightnessRacesinOrbit: thanks, corrected terminology (s/casting/conversion/). – lorro Jan 14 '17 at 19:16
1

`f("abc");` will never call a `f(std::string&)`, even without overloads, because you cannot bind a temporary to a non-const ref., you should add a `const`. – Holt Jan 14 '17 at 19:18
Thanks for replying but the above doesn't work for my implementation. – mkanakis Jan 14 '17 at 19:34
@Davelis4: what exactly doesn't work? Adding a constructor that takes `const char*`? – lorro Jan 14 '17 at 20:23

score 2 · Accepted Answer · answered Jan 14 '17 at 19:16

2

As a rule of thumb, whenever your API exposes overloaded functions or constructors of this signature (or its CV qualified and/or reference variants):

void func(std::string){}
void func(bool){}

You should always provide the one for const char*,

void func(std::string){}
void func(bool){}
void func(const char* c){ func(std::string(c)); }

else you and your users (especially) may be in for subtle suprises because:

string-literals (e.g "abc...") decays quickly to const char* and by overload resolution, conversion to bool has a higher precedence than a user-defined conversion which is what std::string does.

answered Jan 14 '17 at 19:16

WhiZTiM

21,207
4
43
68

1

Depending on what you're doing, you may want to have `func(std::string)` call `func(const char*)` instead, via `.c_str()`. e.g. in the OP's example, I see no need for string construction. – Lightness Races in Orbit Jan 14 '17 at 19:24
1

This works, const char* constructor is called. But as Lightness mentioned that removes the need for string construction. Thanks both. – mkanakis Jan 14 '17 at 19:33

Wrong constructor is being called

2 Answers2