www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Class inside a Struct?

reply "chardetm" <chardet gmail.com> writes:
Hello everyone,

I am currently learning D by coding a project, but I encountered
a problem with one of my structures. I managed to reduce the code
to the minimum:




import std.stdio;
import std.container.rbtree;

struct Container {
	
	private RedBlackTree!int _rbtree = new RedBlackTree!int;

	void add (int elt) {
		_rbtree.insert(elt);
	}

	void print () {
		if (_rbtree.empty) {
			writeln("empty");
		} else {
			foreach (l; _rbtree) {
				write(l, " ");
			}
			writeln();
		}
	}

}

int main () {
	Container c1, c2;
	c1.add(1);
	c1.print();
	c2.print();
	return 0;
}




I don't understand why the output of this program is:

1
1


I expect it to be:

1
empty


I thank you in advance for your help!
Jan 30 2015
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 01/30/2015 11:59 AM, chardetm wrote:


 struct Container {

      private RedBlackTree!int _rbtree = new RedBlackTree!int;


I think you are expecting the new expression to be be executed for every 
object individually. It is not the case: That new expression determines 
the initial value of the _rbtree for every single object of type 
Container. As a result, they will all be sharing the same tree.

The best solution is

1) Remove the new expression:

     private RedBlackTree!int _rbtree;

2) Use a static opCall:

     private RedBlackTree!int _rbtree;

     static Container opCall()
     {
         Container container;
         container._rbtree = new RedBlackTree!int;
         return container;
     }

3) And then initialize the variables with the following syntax:

     auto c1 = Container();
     auto c2 = Container();

Importantly, do not use the following syntax:

     Container c1;    // Warning: c1._rbttree is null

So the safe solution is (used to be?) to disable the default constructor:

      disable this();

However, the compilation fails with the following error message:

Error: struct deneme.Container static opCall is hidden by constructors 
and can never be called

That looks like a bug to me because I am explicitly disabling the 
default constructor. Right?

In any case, an alternative solution is not to use opCall() but do what 
the first part of the second part of the same error message recommends:

Please use a factory method instead, or replace all constructors with 
static opCall.

Yes, you can have factory method that returns a Container object. The 
whole program with that second method:

import std.stdio;
import std.container.rbtree;

struct Container {
     private RedBlackTree!int _rbtree;

      disable this();               // <-- disabled

     this(RedBlackTree!int rbtree)  // <-- good practice
                                    // ("parameterize from above")
     {
         _rbtree = rbtree;
     }

     void add (int elt) {
         _rbtree.insert(elt);
     }

     void print () {
         if (_rbtree.empty) {
             writeln("empty");
         } else {
             foreach (l; _rbtree) {
                 write(l, " ");
             }
             writeln();
         }
     }

}

Container makeContainer()          // <-- factory method
{
     return Container(new RedBlackTree!int);
}

int main () {
     auto c1 = makeContainer();
     auto c2 = makeContainer();

     c1.add(1);
     c1.print();
     c2.print();
     return 0;
}

Ali
Jan 30 2015
next sibling parent "chardetm" <chardet gmail.com> writes:
Thanks a lot Ali, now it works perfectly!

It is quite hard to get used to D's logic, I have to stop 
thinking in terms of C++...

Anyway, thanks again!
Jan 30 2015
prev sibling parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
On 1/30/15 5:28 PM, Ali Çehreli wrote:

 On 01/30/2015 11:59 AM, chardetm wrote:

  > struct Container {
  >
  >      private RedBlackTree!int _rbtree = new RedBlackTree!int;

 I think you are expecting the new expression to be be executed for every
 object individually. It is not the case: That new expression determines
 the initial value of the _rbtree for every single object of type
 Container. As a result, they will all be sharing the same tree.

 The best solution is

 1) Remove the new expression:
 2) Use a static opCall:


Why not use this() ?
Jan 30 2015
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 01/30/2015 01:28 PM, Ary Borenszweig wrote:


 On 1/30/15 5:28 PM, Ali Çehreli wrote:

 On 01/30/2015 11:59 AM, chardetm wrote:

  > struct Container {
  >
  >      private RedBlackTree!int _rbtree = new RedBlackTree!int;

 I think you are expecting the new expression to be be executed for every
 object individually. It is not the case: That new expression determines
 the initial value of the _rbtree for every single object of type
 Container. As a result, they will all be sharing the same tree.

 The best solution is

 1) Remove the new expression:
 2) Use a static opCall:


 Why not use this() ?


In fact, I think a better solution is to use a constructor that takes 
the tree:

     this(RedBlackTree!int rbtree)  // <-- good practice
                                    // ("parameterize from above")
     {
         _rbtree = rbtree;
     }

However, I thought that OP did not want the users know about that 
member. So, as you say, the next option that comes to mind is to use the 
default constructor:

     this()
     {
         _rbtree = new RedBlackTree!int;
     }

Unfortunately, D does not allow defining the default constructor for 
structs:

Error: constructor deneme.Container.this default constructor for structs 
only allowed with  disable and no body

The reason is, the default constructor happens to be the .init value of 
that type and it must be known at compile time.

Ali
Jan 30 2015
parent Ary Borenszweig <ary esperanto.org.ar> writes:
On 1/30/15 7:29 PM, Ali Çehreli wrote:

 On 01/30/2015 01:28 PM, Ary Borenszweig wrote:

  > On 1/30/15 5:28 PM, Ali Çehreli wrote:
  >> On 01/30/2015 11:59 AM, chardetm wrote:
  >>
  >>  > struct Container {
  >>  >
  >>  >      private RedBlackTree!int _rbtree = new RedBlackTree!int;
  >>
  >> I think you are expecting the new expression to be be executed for
 every
  >> object individually. It is not the case: That new expression determines
  >> the initial value of the _rbtree for every single object of type
  >> Container. As a result, they will all be sharing the same tree.
  >>
  >> The best solution is
  >>
  >> 1) Remove the new expression:
  >> 2) Use a static opCall:
  >
  > Why not use this() ?

 In fact, I think a better solution is to use a constructor that takes
 the tree:

      this(RedBlackTree!int rbtree)  // <-- good practice
                                     // ("parameterize from above")
      {
          _rbtree = rbtree;
      }

 However, I thought that OP did not want the users know about that
 member. So, as you say, the next option that comes to mind is to use the
 default constructor:

      this()
      {
          _rbtree = new RedBlackTree!int;
      }

 Unfortunately, D does not allow defining the default constructor for
 structs:

 Error: constructor deneme.Container.this default constructor for structs
 only allowed with  disable and no body

 The reason is, the default constructor happens to be the .init value of
 that type and it must be known at compile time.

 Ali


Thanks for explanation. I was sure there was some reason why you didn't 
suggest it. It's an unfortunate inconsistency, I think. I don't know why 
`.init` is so important or why the default value of a type has any 
importance at all.
Jan 30 2015