2

I am learning to use objects and inheritance and I'm having some trouble to write the constructors for the derived class.

The base class is a simple 'item' with a title, an author, the year the item was written and some notes. I want that when building the item the title and author fields are mandatore, but the other optional. So I have written a constructor with two of the parameters optional.

// item.hpp

class Item
{
public:
    std::string title;
    std::string author;

    unsigned int year;

    std::string notes;

    Item(const std::string &t,
         const std::string &a,
         const unsigned int y = 0,
         const std::string &n = "")
    : title{t}, author{a}, year{y}, notes{n}
    { };
}

Now I have a derived class: 'Book', and I am trying to write the constructors but keep getting errors that 'call to constructor of Book is ambiguous'

// book.hpp

class Book : public Item
{
public:
    unsigned int pages;
    std::string series;

    // Fill only the fields for Item
    Book(const std::string &t,
         const std::string &a,
         const unsigned int y = 0,
         const std::string &n = "")
    : Item(t, a, y, n)
    { };

    // Fill all the fields for Book
    Book(const std::string &t,
         const std::string &a,
         const unsigned int y = 0,
         const std::string &n = "",
         const unsigned int p = 0,
         const std::string &s = "")
    : Item(t, a, y, n), pages{p}, series{s}
    { };

    // Maybe there's no notes
    Book(const std::string &t,
         const std::string &a,
         const unsigned int y = 0,
         const unsigned int p = 0,
         const std::string &s = "")
    : Item(t, a, y), pages{p}, series{s}
    { };
}

Now, if I try to build a book like so:

Book b("a", "b");

I get the error that the constructor call is ambiguous.

My question is this: How should I solve the problem of 'wanting to have more than one constructor with default parameters so that I don't have to fill all the parameters'?

I think that the first constructor of 'Book' can be removed, as it is a 'subset' of the second one. But I don't know how to handle the third one. Any suggestions?

EDIT: I had also thought of building empty objects and just fill all the members later by b.member = whatever but that seemed not very good.

Thanks in advance.

Improve this question
4
  • The constructors only differ in the auto defaulted values. Why not use one constructor with all of the defaults? Commented Mar 13, 2016 at 19:32
  • more precisely overload resolution doesn't considers defaulted parameters, so WRT overload resolution all the constructors in class Book are same Commented Mar 13, 2016 at 19:35
  • Even if the third one doesn't have the 'n' parameter for the notes? Commented Mar 13, 2016 at 19:40
  • Shouldn't also in Item's constructor be default values wrapped with round brackets instead of braces: Item(const std::string &t, const std::string &a, const unsigned int y = 0, const std::string &n = "") : title(t), author(a), year(y), notes(n) Commented Mar 13, 2016 at 19:41

4 Answers 4

Reset to default
4

All your constructors for the Book class has the same number and type of non-default parameters. This means that when you only provide these, the compiler cannot tell which one of them to use, and will give you an ambiguous call error. You really only need one that has the rest defaulted, as you only would call the other ones when you actually do have something you want to fill in, so try removing the default values from all but one constructor.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

So, the solution would be to remove the first constructor, and remove the default parameters for the third up to 'p' ?
2

This happens because you have 2 or more Book constructors that can be called with Book("a", "b"). This problem isn't directly related to default parameters. This error would happen even if one of the constructors would have default values for all parameters. There should be only one constructor that can be called with two strings.

To demonstrate that this problem isn't related to the number of non-default parameters, this would cause the same error because these constructors can also be called with two strings:

// Can be called with two string args:
Book(const std::string &t = "",
     const std::string &a = "")
: Item(t, a, 0, "")
{ };

// This can also be called with two string args:
Book(const std::string &t,
     const std::string &a = "")
: Item(t, a, 0, "")
{ };

From a design perspective your 3 constructors seem redundant, they have more or less the same parameters with different ordering. In your case the solution would be using only one constructor that has the most parameters with all possible arguments combined. Order the default arguments so that the most frequently used ones are at lower indexes in the argument list.

I personally hate functions and constructors with a lot of parameters. If the number of parameters reaches about 5 or more than I usually recommend the following pattern:

Create a struct that contains all parameters for the function call as member variables. Pass a const ref to the struct as the only parameter to the constructor or function:

struct SBookInfo
{
    std::string t;
    std::string a;
    unsigned int y;
    std::string n;

    SBookInfo()
    {
        y = 0;
    }

    // TODO: provide static NAMED factory
    // methods that don't have thousands of arguments
    static SBookInfo CreateWhatever(unsigned int _y=0)
    {
        SBookInfo info;
        info.a = "whatever";
        info.t = "woof";
        info.y = _y;
        return info;
    }
}

// Book constructor
Book(const SBookInfo& info);

This way people who use your code don't have to worry about parameter order and stuff like that and the code that constructs Book instances becomes more clear and readable everywhere:

// With this pattern you can give default value to any args.
// The user of the constructor can specify the args in any order.
SBookInfo info;
info.y = 6;
info.a = "woof";
info.t = "woof";
Book book(info);

// For some special cases the info struct can have NAMED
// factory methods that makes the code more readable.
Book book2(SBookInfo.CreateWhatever(5));

// The above code is much more obvious to read than
// the original. By reading this code who could tell
// me the name of the 3rd argument where we pass 6 to the ctor?
// Book book("woof", "woof", 6);
// And it would become even cleaner with growing number of args.

Another advice: never use names like a, t and so on. Avoid even less simple shorthands. It just isn't worth saving on typing a few chars. Anyway, with modern IDEs you have to type the name only the first time you name the identifier, later autocompletion helps you to avoid typing.

Improve this answer

9 Comments

May I ask: is there a difference with Book b(); b.title = "woof"; b.author = "woof" ?
@Noxbru The member variables (data) of your classes should never be public. That is an obvious design mistake. If they weren't public, you wouldn't be able to write b.title = "woof".
Oh, I am used to C, which doesn't have public/private in structures.
@Noxbru Be very strict. Make everything (data, methods) private by default (especially data member variables) and put them to protected and public only if necessary. In case of member variables they should be private in at least 90% of the cases, sometimes protected, but almost never public. If it has to be accessed from outside then create accessor methods. Let's assume you have a private m_Name member variable in your class. You can create a public const std::string& GetName() const method for it and optionally a void SetName(const std::string& name) if it isn't readonly.
@FrancisCugler That is a common pattern, but this is both const and static that makes it pretty different from a member variable. It is basically a constant inside the namespace of the class that isn't affected by the state and behavior of class instances (encapsulation). I think its better to leave our newbie to play around with the basic stuff before confusing him with more complex stuff. This question has been answered and it isn't a good idea to mix in other things, it just renders this page less useful and readable for others who later find this stackoverflow question.
|
1

W/ delegating constructors:

Book(const std::string &t,
     const std::string &a)
: Book(t, a, 0, "")
{ }

Book(const std::string &t,
     const std::string &a,
     const unsigned int y,
     const unsigned int p = 0,
     const std::string &s = "")
: Book(t, a, y, "", p, s)
{ }

Book(const std::string &t,
     const std::string &a,
     const unsigned int y,
     const std::string &n,
     const unsigned int p = 0,
     const std::string &s = "")
: Item(t, a, y, n), pages{p}, series{s}
{ }

W/o delegating constructors:

Book(const std::string &t,
     const std::string &a)
: Item(t, a), pages{0}, series{""}
{ }

Book(const std::string &t,
     const std::string &a,
     const unsigned int y,
     const unsigned int p = 0,
     const std::string &s = "")
: Item(t, a, y), pages{p}, series{s}
{ }

Book(const std::string &t,
     const std::string &a,
     const unsigned int y,
     const std::string &n,
     const unsigned int p = 0,
     const std::string &s = "")
: Item(t, a, y, n), pages{p}, series{s}
{ }
Improve this answer

5 Comments

Doesn't delegating constructor use the syntax like using Book::Book;?
@AngelusMortis no it dosn't
crap I confused it with inherited constructors, sorry
If in the third constructor I add a default parameter for 'y' and 'n' and I remove the default parameter in the second for 'p', can I remove the first constructor?
@Noxbru yes, there's still room to simplify it
0 
class Item {
protected:
    std::string mTitle;
    std::string mAuthor;
    std::string mNotes;
    unsigned int mYear;

public:
    Item( const std::string& title, const std::string& author, unsigned int year = 0, const std::string& notes = std::string() );        

};

Item::Item( const std::string& title, const std::string& author, unsigned int year, const std::string& notes ) :
mTitle( title ),
mAuthor( author ),
mYear( year ),
mNotes( notes ) {
} // Item

class Book : public Item {
private:
    std::string mSeries;
    unsigned int mPages;

public:
    Book( const std::string& title, const std::string& author, unsigned int year = 0, unsigned int pages = 0,  const std::string& series = 0, const std::string& notes = std::string() );       

};

Book::Book( const std::string& title, const std::string& author, unsigned int year, unsigned int pages, const std::string& series, std::string& notes ) :
Item( title, author, year, notes ),
mSeries( series ),
mPages( pages ) {
} // Book

Now I did make the members of Item protected so that any derived class such as Book has direct access to them, and the members in Book I made private, so you would need accessor functions or methods to modify or retrieve these members from outside objects or code sources.

Edit In the Book's constructor I changed the order of parameters slightly. I moved the number of pages towards the left and moved the notes farther to the right of the parameter calls for default parameters. I did this because it is more likely to call this constructor with the number of pages available before there are notes available, and I also moved the series and left the notes as a last parameter.

int main() {
    // Different Ways To Call this using default parameters
    Book b1( "Fellowship of the Ring", "Tolkien", 1958, 387, "Lord of the Rings" ); // No Notes

    Book b2( "Apostle of John" "John", 27, 80, std::string() or " ", "A book of the New Testament found in the Holy Bible" ); // Notes, but not series.

    return 0;
} // main
Improve this answer

2 Comments

Sorry, but I'm afraid that this is not the same. My third constructor doesn't have the 'n' parameter.
@Noxbru It is a defaulted parameter. If the person doesn't have notes available to add to the constructor then they can omit it, or if they don't have notes but they do have series which follows it then they would call the constructor like this: Book b1( "Fellowship of the Ring", "Tolkien", "1958", 387, " ", "Lord of the Rings" );

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.