Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and NRVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and NRVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and NRVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)
deleted 5 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and names RVONRVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and names RVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and NRVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

Please don't do this:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

I would change a couple of things:

###In WidgetBuilder I don't like the explicit call to create() WidgetBuilder.

unique_ptr<Widget> create()
{ return make_unique<Widget>(m_name); }

I would replace it with a conversion operator:

operator std::unique_ptr<Widget>()
{    return std::make_unique<Widget>(m_name);}

Then usage becomes:

unique_ptr<Widget>  val = WidgetBuilder().name("Loki");

###In WidgetContainer

I don't see the need to put the std::vector inside a std::uniqu_ptr.

unique_ptr<vector<unique_ptr<Widget>>> m_widgets;

I would just do

std::vector<std::unique_ptr<Widget>>> m_widgets;

Because of RVO and names RVO this is very efficient when returning objects. Also with the use of "Move Semantic" even passing it as a parameter is now very efficient.

###In WidgetContainerBuilder

Like WidgetBuilder remove the create() method.

Not sure I would pass WidgetBuilder to the addWidget() method. Why not a std::unique_ptr<Widget> create them on the fly and use them as needed. (I suppose this is why you had an explicit create() method on Widget.

WidgetContainerBuilder &addWidget(WidgetBuilder &widgetBuilder)