views:

296

answers:

5

I guess I've asked a few similar questions before, but I was beating around the bush. I think this is the real problem that I can't quite lay to rest.

I'm dealing with a third party library, and there's an object that can't create itself, b2Body. The b2World has to instantiate it. I personally don't like this design pattern very much; I think the b2Body should be able to exist independently of the world, and then be added to the world when needed. Anyway, I've wrapped b2Body with my own class class, Body, because I need to add some extra stuff to it anyway. Similarly, I have a World wrapper. Now I figure I have 3 options:

  1. Have Body's constructor takes a pointer to World so that it can be fully instantiated (calls b2World::CreateBody somewhere inside) -- i.e. have a constructor like Body *b = new Body(world_ptr)
  2. Pass Body to some World::CreateBody method like how the library already does it -- i.e. Body *b = world.CreateBody(params);
  3. Duplicate all the data in b2Body so that you can use it however you want, and then after you add it to the world it will 'switch over' to use the b2Body data -- i.e. Body b and later world.addBody(b).

(1) and (2) mean that you can't have a Body without a World, which I probably won't need, but it might be nice to have that option [so that I can use it as a template for other objects and such]. Not sure what other pros and cons there are. (3) seems nicer, but it's a lot more work to implement, and it means I have to duplicate most of the data that's already contained in b2Body.

What are your thoughts? I'll CW this just so no one frets.


I still can't lay this to rest. This is what each of the options would look like:

Option 1: (what I prefer)

World w;
Body b;
Fixture f;
b.addFixture(f);
w.addBody(b);

Option 2: (somewhere in the middle)

World w;
Body b(w);
Fixture f(b);

Option 3: (how Box2D does it)

World *w = new World;
Body *b = w->CreateBody(args);
Fixture *f = b->CreateFixture(args);

Options 2 and 3 aren't so different, but it changes who has control over is creating the objects.

How would I actually implement option 3 though? World::CreateBody() has to call b2World::CreateBody(args) which calls b2Body::b2Body() and returns b2Body but never calls Body::Body(args) which is a problem. The b2Body would get fully initialized, but my wrapper has no place to do it's thing... More specifically, how would I write World::CreateBody(const BodyDef &bd)? Assuming BodyDef inherited from b2BodyDef, Body from b2Body, World from b2World, etc.

+2  A: 

Sounds like the b2World object is a factory for b2Body, so the author has decided that a b2Body has no meaning without its world.

My first reaction would be that this is the interface, so live with it. Have your World object be a factory for your Body. So that's close to approach (1) except that you don't have a public constructor, the World object has a makeBody() method.

You think that Bodies without World make sense? If so, perhaps what you find is that some subset of Body methods could be useful without a World, I'm not clear how you implement them - they clearly can't be implemented by b2Body, because he can't exist without a b2World. So one possibility is that you have a set of config information

 class Body {
        int howBig;
        String name;
        Flavour flavour;
        // and getter/setters
 }

Now these (or at east the bgetters) clearly could make sense with or without World.

With that in mind, I think you may find that you actualy have two "states" of Body, one when it's not associated with World, one when it is. And the actual capabilities are different. Hence you actually have two interfaces.

So have a IndependentBody class and a Body class. The World factory method might have a signature

World {

    Body makeBody(IndependentBody);

}
djna
I think that's like (2) actually. "IndependentBody" would be like Box2D's b2BodyDef. I was hoping to remove one layer of abstraction.
Mark
+5  A: 

I think, if you're going to use a third-party library, you should only fight its design if you have a much better reason than oh, I don't like that design pattern much. Your library has a way of doing things — apparently, by using a factory object — and fighting that will increase your code complexity, possibly substantially.

derobert
Well... it was slightly more than that. I'm using both Qt and Box2D, and Qt does things the way I suggested. This would keep some consistency across the project. But by adopting Box2D's way of doing things, suddenly I have two different design patterns going on. I guess it's not such a big deal and I shouldn't resist it.
Mark
+1  A: 

I agree that you shouldn't be fighting the design of a 3rd party library that you're using. Heading down such a path can cause a lot of problems in the future.

By looking "under the covers" and creating wrappers, you may be locking down the behavior of the 3rd party library to the way in which the current implementation behaves.

What will happen if a future version of the API stays the same, but the underlying semantics change?

Suddenly, everything is broken from the point of view of your wrapper.

Just my 0.02.

Rob Wells
+1  A: 

Following your link, I see that createBody doesn't return a b2Body, but a pointer to one:

 b2Body* b2World::CreateBody  ( const b2BodyDef*  def );

This is likely because b2World

  1. manages the b2Body lifetime (i.e., deletes it and the memory it uses when the B2World goes out of scope/is itself deleted), or

  2. Because the B2Wsorld needs to maintain pointers to b2Bodies, e.g. to iterate over them to accomplish some B2World functionality.

I also note the all that's required (other than a b2World) to create a b2Body is a pointer to a b2BodyDef.

So if you want a b2Body that's not attached to a b2World, but can at some later be attached to one, why not pass around b2BodyDefs, or pointers to them?

I might create a thin wrapper for a b2BodyDef, e.g.,:

 class b2BodyDefWrapper {
   public const b2BodyDef& b2bodyDef;
   public b2BodyDefWrapper( const b2BodyDef& bodydef ) : b2bodyDef(bodydef) {}
   public const b2Body* reifyOn( b2World& world) const { 
     return world.CreateBody( b2bodyDef ) ;
   }
 }

Note that I could attach this b2BodyDefWrapper to multiple worlds, or to the same world more than once.

Now it may be that you can do things to a b2Body that you can't do to a b2BodyDef, and so that passing around (possibly wrapped) b2BodyDefs won't suit your purposes. In this case, I might use the Command Pattern to "attach" a list of functions to the b2BodyDefWrapper , that would be "replayed" on each reified b2Body:

 class b2BodyDefWrapper {
   private std::vector<Command&> commandStack;
   public const b2BodyDef& b2bodyDef;
   public b2BodyDefWrapper( const b2BodyDef& bodydef ) : b2bodyDef(bodydef) {}
   public const b2Body* reify( b2World& world) const { 
     b2body* ret = world.CreateBody( &b2bodyDef ) ;
     for (int i=0; i< commandStack.size(); i++) {
        v[i].applyTo( ret ) ;
     }
     return ret;
   }

   public void addCommand( const Command& command ) {
      commandStack.push_back( command );
   }
 }

Where Command is an abstract base class for Functors, like this:

  class Command {
     virtual ~Command() {}
     virtual void applyTo( b2Body* body ) = 0 ;
  }

with concrete subclasses:

 class ApplyForce : public Command {
   private const b2Vec2& force;
   private const b2Vec2& point;
   ApplyForce(const b2Vec2& f, const b2Vec2& p) : force(f), point(p) {}
   virtual void applyTo( b2Body* body ) {
      body->ApplyForce( force, point ) ;
   }
 }

Then I could use my wrapper like this:

extern b2BodyDef& makeb2BodyDef();
b2BodyDefWrapper w( makeb2BodyDef()  ) ; 
ApplyForce a( ..., ... );
w.addCommand( a ) ;
...
b2World myworld;
b2World hisWorld;
w.reifyOn( myWorld ) ;
w.reifyOn( hisWorld) ;

Note that I've left out some details, principally about object ownership and memory management, and who calls delete on CommandStacks; I also haven't followed the rule-of-three in my sketches of the classes. You can fill in these as you like.

I also have left out any provision for calling, from a Command, b2Body functions that return other than void and returning those values; you can probably cover this (should you need to) by having ApplyTo return a union of some sort.

More fundamentally, I haven't covered how one concrete Command can supply its return value (if any) to another concrete Command. A full solution would be to have not a Vector of Commands, but an n-ary tree of them, where child Commands are applied first, and their return values (if any) are supplied to their parent Command. Whether you need such complexity is a question I obviously can't answer. (And I've already given a pretty detailed answer, considered I'm neither getting paid for this, nor am I getting reputation point, since you Community Wiki'd this question.)

tpdi
Well, it's #2. The rest of this I'll have to absorb... I'm not sure how I feel about your command structure. I'm trying to simplify things :)
Mark
+1  A: 

One reason that box2D uses bodyDef objects to construct b2Body objects is so that you can re-use the def to create multiple bodies. Code like:

b2BodyDef myDef;
// fill out def

for (int i=0; i < 100; ++i) {
   for (int j=0; j < 100; ++j) {
      myDef.x = i;
      myDef.y = j
      b2Body* body = world.CreateBody(myDef)
   }
}

Is a very efficient and compact way of creating many objects with the same characteristics. It doesnt have to be in the same loop either, you can keep the def objects around as metadata, and create bodies from them as needed.

Don't fight it because it is there for a reason.

sean riley
You could do that without the body def though, just by cloning and modifying a few properties.
Mark
not sure if that is true.. the b2Body must exist in a world, so cloning it and modifying properties could have side-effects inside the world where the body is cloned. It could generate collision/contact events at the original position rather than where it will be after the properties are set?Also that doesn't address keeping a def around as a template. Keeping an actual b2Body around as a template to clone from makes less sense. What world is it in? What if something collides with it? Is it simulating?
sean riley
Well hang on, I was proposing that you simply wouldn't add it to the world in that case. This is exactly a situation where it makes sense to have a body that doesn't belong to a world. Also, assuming there aren't any multi-threaded shannanigans going on, presumably you would finish updating it before the next physics step is run so collisions wouldn't be an issue.
Mark
to be pedant about the design pattern, in this context a b2Body is a "physics object in a world", and a bodyDef is a template that an object can be created from. The library has separated these two responsibilities, and trying to use the wrong class for the wrong purpose is just wrong.
sean riley