1

I have some code like that:

#include <string>

class another_foo
{
public:
    another_foo(std::string str)
    {
        // something
    }

private:
    // something
};

class foo
{
public:
    foo();

private:
    another_foo obj;
};

foo::foo() : obj(str) // no `: obj("abcde")`, because it is not that simple in real situation.
{
    std::string str = "abcde"; // generate a string using some method. Not that simple in real situation.
    // do something
}

and I am going to initialize obj which is a private member of foo. But this code does not compile. How can I use the variable in the constructor's body in the initialization list?

AFAIK, the only method is to separate the code generating str from the constructor as another function, and then call that function directly in the initialization list. That is...

#include <string>

class another_foo
{
public:
    another_foo(std::string str)
    {
        // something
    }

private:
    // something
};

class foo
{
public:
    foo();

private:
    another_foo obj;

    // std::string generate_str() // add this
    static std::string generate_str() // EDIT: add `static` to avoid using an invalid member
    {
        return "abcde"; // generate a string using some method. Not that simple in real situation.
    }
};

foo::foo() : obj(generate_str()) // Change here
{
    // do something
}

But is there any better method?

qpalz
  • 71
  • 2
  • 7

4 Answers4

3

Yes, you have to move it to a function. If it's a single-purpose thing (only used for this initialisation) and you have access to C++11 lambdas, you can use a single-purpose lambda. Otherwise, just use a member function like you did. Just be careful about calling virtual functions in there, because the object is still under construction. Best make it static, if possible.

Lambda example:

class foo
{
public:
    foo();

private:
    another_foo obj;
};

foo::foo() : obj([] { return "abcde"; } ())
{
    // do something
}
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • That's the answer I want :) Lambdas is not good enough in this situation as the function is actually quite long. It seems that using a static member function is the best. – qpalz Jun 20 '13 at 12:12
2

But is there any better method?

To be short: No, not if you're not willing to change either the way obj is allocated OR it's semantic.

You can do variants of this, like making generate_str() static, or better (if the code is short) using a lambda:

foo::foo() : obj( []{ return "abcde"; }() )
{
}

HOWEVER: If the object construction requires logic which is dependent on other members, then you have to make sure the initialization order of members reflect the inter-dependencies (order them in the declaration from the independant to the dependants) OR BETTER: change the semantic of obj OR allocate it on the heap.

Changing allocation have a cost on construction/destruction and a very minor cost on access, so it's not the best solution most of the time, but it solves the problem:

class foo
{
public:
    foo();

private:
    std::unique_ptr<another_foo> obj; // no sharing of the instance

};

foo::foo() // obj is null
{
    // do something
    auto result_obj_info = compute_something();
    obj = new another_foo( result_obj_info ); 
    // OR if you use this http://stackoverflow.com/questions/12547983/is-there-a-way-to-write-make-unique-in-vs2012
    obj = std::make_unique<another_foo>( result_obj_info );
}

However, I would recommand changing the semantic of another_foo instead so that it have value semantic:

#include <string>

class another_foo
{
public:

    another_foo(); // this create an invalid anoter_foo, unusable.

    another_foo(std::string str) // this create a valid another_foo
    {
        // something
    }

    // make sure copy/move functions are available, either automatically or manually

    bool is_valid() const;

private:
    // something
};

inline bool is_valid( const another_foo& obj ) { return obj.is_valid(); }

class foo
{
public:
    foo();

private:
    another_foo obj;
};

foo::foo()
{
    assert( is_valid( obj ) == false);
    std::string str = "abcde"; // generate a string using some method. Not just simple like that in real situation.
    // do something
    obj = another_foo( str );
    assert( is_valid( obj ) == true);
}

That way your another_foo type acts like a handle for it's resources. If it shouldn't be copied, just make it move-only. Take a look at how std::thread or std::unique_ptr work for example.

Klaim
  • 67,274
  • 36
  • 133
  • 188
  • The second method is what I used to use, but I think this is quite ugly :( That's why I raise this question here. The first method is quite good :) – qpalz Jun 20 '13 at 12:15
  • @qpalz You mean that in your real case the unique_ptr version is better? It really depends on what your another_foo is. The pointer version will require allocating it separately which can be a problem or not. – Klaim Jun 20 '13 at 12:39
  • No. In the real case, using lambda is better. I mean that I used to use unique_ptr but I think it is ugly, and there will be a slight overhead. – qpalz Jun 20 '13 at 12:49
  • @qpalz Yes that's globally what I was suggesting, but as I said in my answer, lambdas will not work if at any moment you need to initialize your object with computation from other members. So lambda is a good solution for short and isolated computation, but for more complex initialization, use value semantic. – Klaim Jun 20 '13 at 12:54
  • For complex but *isolated* computation, method suggested in my original post may be better, as Angew said? Value semantic is quite complex, I think. – qpalz Jun 20 '13 at 13:01
  • @qpalz I'm surprized because value semantic makes everything quite simpler. For complex but isolated computation, as I was also saying in my answer, yes a static function is better. – Klaim Jun 20 '13 at 13:04
  • Yes, value semantic is the simplest method for complex and the computation depending on the member of the class. Sorry for misunderstanding what you mean :( – qpalz Jun 20 '13 at 13:07
1

The risk when writing a member function that will be called during construction is that you may change it afterwards to use some of the class' data members, which may not have been initialized at the time of the call.

It's probably better to define an external function to generate the string, like this:

namespace {
std::string generate_str()
{
    return "abcde"; 
}
}

foo::foo() : obj(generate_str()) 
{
    // do something
}

That way, if you have to pass parameters to the function, the use of uninitialized data members or virtual member function return values will be visible from the constructor, hence more easy to catch.

slaphappy
  • 6,894
  • 3
  • 34
  • 59
  • This problem can also be solved by making `generate_str()` static, as @Angew says, right? Because it will be quite ugly to make an external function. – qpalz Jun 20 '13 at 12:10
  • ... or just make it a `static` member function. – Angew is no longer proud of SO Jun 20 '13 at 12:11
  • Sure you can make it static, but then it requires you to change the class declaration. Since it's an implementation detail, it should not impact the class' interface. – slaphappy Jun 20 '13 at 12:13
  • @kbok Well... That's not a big problem as my project is only at the initialization stage. Using an external function may violate the aims of using OO, and I think it should be avoided. Using lambdas may be a good method? – qpalz Jun 20 '13 at 12:22
  • Using a lambda may be a good method, only if the initialization function is very short, ie a one-liner. Otherwise it will quickly become very hard to read. Regarding "using OO", remember that 1) C++ is not a OO-only language (and trying to write OO-only code makes it bad), and 2) Hiding implementation details from the interface is more respectful of OO principles than preferring static member functions over free functions - especially since they are technically almost equivalent. – slaphappy Jun 20 '13 at 12:55
0

If it's a constant specific to that class, you can define it that way:

class foo {
    public:
        foo();
    private:
        static const std::string STR;
        another_foo obj;
};

const std::string foo::STR = "abcde";

foo::foo() : obj(STR)
{
}

Edit
Since it seems it's not a constant, you may have to use a static member function for this job. (Or a lambda, your choice)

class foo {
    static std::string generate_string() const;
};

Implementation:

std::string foo::generate_string() const {
    return std::string("abcde");
}
user123
  • 8,970
  • 2
  • 31
  • 52