🚀 go-pugleaf

Mike K wrote:
>   MyClass(int x, int y) {
>     Reset();
>     _x = x; _y = y;

No member named _x or _y. You called them plain old x and y.
I'd recommend using x_ and y_, as names starting with an underscore
are generally reserved for use by the implementation.


-- 
"Come to think of it, there are already a million monkeys
on a million typewriters, and Usenet is NOTHING like
Shakespeare."
	- Blair Houghton

To answer the original question: no, there's no problem with calling
a member function in the body of the ctor.

-- 
"Come to think of it, there are already a million monkeys
on a million typewriters, and Usenet is NOTHING like
Shakespeare."
	- Blair Houghton

"Mike K" <mkipper@rogers.com> wrote:

> This is a student's, not a school question. I'm just learning C++.
> Just a quick example. Remember hearing something about this being wrong, but
> can't rembember.

What you are probably remebering is calling a *pure virtual* 
member-function from inside the c_tor. This cannot be done.

"Jacques Labuschagne" <hardwired@xtra.co.nz> wrote in message
news:owJo9.5131$Os6.704244@news.xtra.co.nz...
> Mike K wrote:
> >   MyClass(int x, int y) {
> >     Reset();
> >     _x = x; _y = y;
>
> No member named _x or _y. You called them plain old x and y.
> I'd recommend using x_ and y_, as names starting with an underscore
> are generally reserved for use by the implementation.

Names containing two consecutive underscores and names starting with an
underscore followed by an uppercase letter are reserved for the
implementation everywhere.
Names starting with an underscore followed by a lower case letter are
reserved for the implementation only in the global namespace.

So in this case the OP is ok, but the rules are hard enough to remember that
always avoiding names starting with an underscore is good advice.

Mike K wrote:

> I can't remember what the problem with calling a member function 
> during construction is (if any). It seems this way would be 
> easier to add another variable to a class, without rewriting all 
> the constructors. Please advise.

The problem that you remember is that of calling virtual
functions inside of a constructor.  You may not get the behavior
that you desire.

-tom!

Mike K wrote:
> This is a student's, not a school question. I'm just learning C++.
> Just a quick example. Remember hearing something about this being wrong, but
> can't rembember. If so, why?
> 
> class MyClass {
>   int x, y, z;
> public:
>   MyClass(void) {
>     Reset();
>   }
> 
>   MyClass(int x, int y) {
>     Reset();
>     _x = x; _y = y;
>   }
> 
>   void Reset(void) {
>     _x = _y = _z = 0;
>   }
> };
> 
> I can't remember what the problem with calling a member function during
> construction is (if any). It seems this way would be easier to add another
> variable to a class, without rewriting all the constructors. Please advise.
> ...

Aside from incorrect member variable names, this particular example doesn't
exhibit any problems.

However, there is a generic design error that is usually illustrated by an
example that uses similar calling pattern. Moreover, I'd say that this
design error is exhibited by your code.

Method 'Reset' is a public method  of the class. This means that this method
can be called form the client code at any time. This, in turn, means that
method 'Reset' can (and will) always assume that the current state of the
'*this' object is a valid state. This should be taken into account when this
method is called form a constructor. Before the method is called it is
important to make sure that the construction steps that have already been
completed have brought the '*this' object to a valid state.

Let's take a look at your code. You call method 'Reset' from class's
constructor. Is the object's state valid at the moment of the call? I'd say
it isn't. The object's data fields contain indeterminate values. In this
particular case you are lucky, because the implementation of method 'Reset'
doesn't rely on the current object state in any way. But what if tomorrow
you (or someone else) decide to change the implementation of this method so
that it starts with dumping the value of 'x * y / z' into a log file and
only then sets all values to, say, 1?

  void Reset()
  {
    assert(z != 0);
    dump(x * y / z);
    x = y = z = 1;
  }

In this case you'll definitely run into problems with the calls that
originate from constructors.

In order to get rid of the problem you should either make sure that the
object is in valid state before the method is called (by explicitly
initializing x, y and z _before_ the call) or break the method into two
separate methods - one is public (assumes that the object is in valid state)
and another is private (makes no such assumption). The constructors should
call the private version, not the public one

  private:

    void Init() // makes no assumptions about the current object state
      { x = y = z = 1; }

  public:

    MyClass()
     { Init(); }

    void Reset() // assumes that the object is in valid state
    {
      assert(z != 0);
      dump(x * y / z);
      Init();
    }

Once again, from the formal point of view, the code from your original post
is fine. Nevertheless, it is a good practice to _always_ assume that public
methods of the class _require_ that the instance of the class is in valid
state at the moment of the call.

-- 
Best regards,
Andrey Tarasevich
Brainbench C and C++ Programming MVP

Thanks everyone for your input. I learn a lot of great things from everyone
in this NG.
But I do think that Andrey's post addressed the issues I'd previously heard
about. I like the idea of the Init() function. I guess the idea was to have
one init function, so if there were 10 variables that always were
initialized to zero no matter which type of constructor was called, I
wouldn't have to init them in each constructor, causing (to me) unreadable,
pages long code.

Thanks again!

Mike


"Andrey Tarasevich" <andreytarasevich@hotmail.com> wrote in message
news:3DA37195.2010605@hotmail.com...
> Mike K wrote:
> > This is a student's, not a school question. I'm just learning C++.
> > Just a quick example. Remember hearing something about this being wrong,
but
> > can't rembember. If so, why?
> >
> > class MyClass {
> >   int x, y, z;
> > public:
> >   MyClass(void) {
> >     Reset();
> >   }
> >
> >   MyClass(int x, int y) {
> >     Reset();
> >     _x = x; _y = y;
> >   }
> >
> >   void Reset(void) {
> >     _x = _y = _z = 0;
> >   }
> > };
> >
> > I can't remember what the problem with calling a member function during
> > construction is (if any). It seems this way would be easier to add
another
> > variable to a class, without rewriting all the constructors. Please
advise.
> > ...
>
> Aside from incorrect member variable names, this particular example
doesn't
> exhibit any problems.
>
> However, there is a generic design error that is usually illustrated by an
> example that uses similar calling pattern. Moreover, I'd say that this
> design error is exhibited by your code.
>
> Method 'Reset' is a public method  of the class. This means that this
method
> can be called form the client code at any time. This, in turn, means that
> method 'Reset' can (and will) always assume that the current state of the
> '*this' object is a valid state. This should be taken into account when
this
> method is called form a constructor. Before the method is called it is
> important to make sure that the construction steps that have already been
> completed have brought the '*this' object to a valid state.
>
> Let's take a look at your code. You call method 'Reset' from class's
> constructor. Is the object's state valid at the moment of the call? I'd
say
> it isn't. The object's data fields contain indeterminate values. In this
> particular case you are lucky, because the implementation of method
'Reset'
> doesn't rely on the current object state in any way. But what if tomorrow
> you (or someone else) decide to change the implementation of this method
so
> that it starts with dumping the value of 'x * y / z' into a log file and
> only then sets all values to, say, 1?
>
>   void Reset()
>   {
>     assert(z != 0);
>     dump(x * y / z);
>     x = y = z = 1;
>   }
>
> In this case you'll definitely run into problems with the calls that
> originate from constructors.
>
> In order to get rid of the problem you should either make sure that the
> object is in valid state before the method is called (by explicitly
> initializing x, y and z _before_ the call) or break the method into two
> separate methods - one is public (assumes that the object is in valid
state)
> and another is private (makes no such assumption). The constructors should
> call the private version, not the public one
>
>   private:
>
>     void Init() // makes no assumptions about the current object state
>       { x = y = z = 1; }
>
>   public:
>
>     MyClass()
>      { Init(); }
>
>     void Reset() // assumes that the object is in valid state
>     {
>       assert(z != 0);
>       dump(x * y / z);
>       Init();
>     }
>
> Once again, from the formal point of view, the code from your original
post
> is fine. Nevertheless, it is a good practice to _always_ assume that
public
> methods of the class _require_ that the instance of the class is in valid
> state at the moment of the call.
>
> --
> Best regards,
> Andrey Tarasevich
> Brainbench C and C++ Programming MVP
>