Skip to main content
gamedev.net
🔒 Locked

big and complex function

Started by luasitdown 4 replies 350+ views
Original Post
luasitdown
luasitdown
are there some way to do big and complex function? I have done a function D3DXVECTOR3 collideresponse(Movable movable) it judge if movable collid with other (wall etc...) if collided, stop,if move again stop. if move back ,go on. I found this complex function is hard to handle. there are several flow and easy to create bug. much variable in it are relative,it is not easy to divide to several function. So some available experience to do?
Rebooted
Rebooted
You should be able to split it into smaller functions quite easily, by seperating the operations, passing variables as arguments, etc. Look up refactoring. Post your code if you want help with your specific problem.

Edit: the function you described should probably delegate a lot of its work to the object it is operating on and other subsystems.
choffstein
choffstein
Break the function down into as many small pieces as possible.
Some pieces may be "didCollide()", "stopObject()", "moveObject()", et cetera.
Modularize your code into those other functions, and it should make it much easier to think about and organize instead of long lines of spaghetti code.

Good luck.
MaulingMonkey
MaulingMonkey
Quote:
Original post by luasitdown
much variable in it are relative,it is not easy to divide to several function.


It's probably easier than you think.

If you have a lot of "context" data, this can be factored into class groupings. Instead of:

void render( void ) {    Handle foo = SomeAPIFunction();    HANDLE bar = someOtherAPI();    window_handle baz = i_like_pie_api();    //do lots of things using foo, bar, and baz}


You can group things together in a single grouping which is easier to pass around:

struct render_context {    Handle foo;    HANDLE bar;    window_handle baz;};void render( void ) {    render_context context;    context.foo = SomeAPIFunction();    context.bar = someOtherAPI();    context.baz = i_like_pie_api();    do_some_rendering( context );    do_some_more_rendering( context );    do_the_remaining_rendering( context );}void do_some_rendering( render_context & context ) {    //use context.foo, .bar, and .baz}void do_some_more_rendering( render_context & context ) {    //use context.foo, .bar, and .baz}void do_the_remaining_rendering( render_context & context ) {    //use context.foo, .bar, and .baz}


Of course, it'd be even saner to organize these functions into a single cohesive unit - since the do_*_rendering functions don't make sense at all without their context, we can just make them member functions - renaming it to a "renderer" since it will now do more than just hold the context data - plus you don't have to add "context." in front of every instance of foo, bar, and baz!:

struct renderer {    Handle foo;    HANDLE bar;    window_handle baz;public:    renderer() {        foo = SomeAPIFunction();        bar = someOtherAPI();        baz = i_like_pie_api();    }    void render( void ) {        do_some_rendering();        do_some_more_rendering();        do_the_remaining_rendering();    }    void do_some_rendering( void ) {        //use foo, bar, and baz    }    void do_some_more_rendering( ( void  ) {        //use foo, bar, and baz    }    void do_the_remaining_rendering( ( void  ) {        //use foo, bar, and baz    }};void render( void ) { //for backwards compatibility    renderer my_renderer;    my_renderer.render();}


OOP for the win!!
luasitdown
luasitdown
D3DXVECTOR3 *Collision::isCollide( D3DXVECTOR3 *out, Entity * movable, Quad *wall){	static Aabb oldAabb = movable->worldAabb();	static D3DXVECTOR3 oldPos = movable->node()->position();	static bool isLastCollided = false;	//get position	*out = movable->node()->position();	//get wall plane	Plane &plane = *wall->equation();	// abbr. aabb	Aabb  aabb = movable->worldAabb();	// output last frame if the movable collide with wall	show(isLastCollided);	// movable move direction during timeDelta that from last frame to the follow frame	D3DXVECTOR3 newDirection = aabb.m_max - oldAabb.m_max;	// last frame movable move direction	static D3DXVECTOR3 lastMoveDirection = newDirection;	// dot product last direction and the next direction to check the angle between two direction 	// to see movable  move ahead or backwards 	// if move ahead and collision happen last frame stop movable.	// if move backwards ,set isLastCollided false, let movable be.	float dot = D3DXVec3Dot(&lastMoveDirection, &newDirection);	// track precious in dot product	show(dot);	assert(dot==0);//	if(dot!=0)MessageBox(0,"test",0,0);//	show(&newDirection);	//if movable move ahead or do not move return last frame position that is stop it.	if( dot > .02 || 		D3DXVec3Length(&newDirection) < std::numeric_limits<float>::epsilon() )	{		if(isLastCollided)		{			*out = oldPos;			lastMoveDirection = newDirection;			return out;		}		else		{		}	}	//if move backwards set isLastCollided false.	else if  (dot < -.02)	{		isLastCollided = false;	}	// if move parallel ,set true   this maybe not necessary	else if  (abs(dot) < .02 && D3DXVec3Length(&newDirection) > std::numeric_limits<float>::epsilon())	{		show(dot);		isLastCollided = true;	}	else	{	}	// 8 vertices of moving aabb construct 8 ray	Ray  ray[8];	// store 8 vertices of aabb	D3DXVECTOR3 endPoint[8];	// intersect result 	typedef std::vector<float>/**/ resultType;	resultType resultVector;	// get 8 ray plane intersect result because of 8 rays 	for (int k = 0;k<8;k++)	{		// get 8 rays on aabb.		ray[k].setRay ( oldAabb.corner(k), aabb.corner(k) );		endPoint[k] = aabb.corner(k);		//ray plane intersect test		std::pair<bool,float> result = ray[k].intersects( plane );		// if false that is parallel return last position		if ( !result.first )		{			oldAabb = aabb;				*out = oldPos = movable->node()->position();			return out;		}		//store test result		resultVector.push_back(result.second);		//std::cout<<result.first<<'\n';	}	// the follow ,get the vertex of aabb that is closest to wall	//找出最小值  得到 对应的 ray  	//intersect 结果 和 ray 对应的	resultType::iterator iter = std::min_element( resultVector.begin(), resultVector.end() );	// [ index得到了 ]	int index = iter - resultVector.begin();	// vertex of aabb that is closest to wall and intersect vertex construct  a ray	D3DXVECTOR3 intersectRay = ray[index].tPoint(*iter) - ray[index].m_origin; 	D3DXVECTOR3  walkRay = endPoint[index] - ray[index].m_origin; 	/* if t>0 (towards to wall) or walkRay length < length of intersectRay(pass through wall)	Intersect actually ignore normal sign of wall*/	if ( D3DXVec3Length(&intersectRay) < D3DXVec3Length(&walkRay) && resultVector[index] > 0 )	{		*out = intersectRay + oldPos;		lastMoveDirection = walkRay;		isLastCollided = true;	}	// keep the new position	oldAabb = aabb;		oldPos = movable->node()->position();	lastMoveDirection = newDirection;	return out;}

one of big bad thing I think is static ,it make split to smaller function to be difficult,do you think so?
another big thing is condition is much, it is hard to manage.
If you are interesting , you could look my code, see where need to improve or change.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.