Back to General and Gameplay Programming

Would this be considered duplicate code?

General and Gameplay Programming Programming

Started by Nicholas Kong January 23, 2015 03:49 AM

8 comments, last by jbadams 9 years, 2 months ago

Nicholas Kong

1,535

Author

January 23, 2015 03:49 AM

From what I can gathered about duplicate code, it is if the list of code that occurs more than once. Okay, based on what context? Is a duplicate code based on the similar appearance of the code?

In the below case, I have a character state animation driven by key press events and enum that hold states or behavior denoted by constants.

The reason is the first and second code are the similar in appearance and in functionality but they used different state and key presses to execute this functionality of the game character.

if(direction == Direction.RIGHT && state == ActionState.RUNNING)
{
runRightAnim.update();
 
if(rightReleased)
{
state = ActionState.IDLE;
}
 
}
 
if(direction == Direction.LEFT && state == ActionState.RUNNING)
{
runLeftAnim.update();
 
if(leftReleased)
{
state = ActionState.IDLE;
}
}

L. Spiro

25,818

January 23, 2015 03:57 AM

“state == ActionState.RUNNING” is certainly duplicate.
I have to go or else I would mention more, but a very quick rewrite:

if ( state == ActionState.RUNNING ) {
	bool bIdle = false;
	// switch/case also possible.
	if ( direction == Direction.RIGHT ) {
		runRightAnim.update();
		if ( rightReleased ) {
			bIdle = true;
		}
	}
	else if ( direction == Direction.LEFT ) {
		runLeftAnim.update();
		if ( leftReleased ) {
			bIdle = true;
		}
	}
	// …
	// OTHER CHECKS THAT COULD CAUSE IDLE STATE.
	
	if ( bIdle ) {
		state = ActionState.IDLE;
	}
}

L. Spiro

I restore Nintendo 64 video-game OST’s into HD! https://www.youtube.com/channel/UCCtX_wedtZ5BoyQBXEhnVZw/playlists?view=1&sort=lad&flow=grid

luveti

589

January 23, 2015 04:01 AM

In this case you could remove a lot of that code. Here's my refactor of it:


if(state == ActionState.RUNNING) {
    if(direction == Direction.LEFT) {
        runLeftAnim.update();
    } else if(direction == Direction.RIGHT) {
        runRightAnim.update();
    }

    if(leftReleased || rightReleased) {
        state = ActionState.IDLE;
    }
}

As you can see I combined the two top level "if(state == ActionState.Running)" statements into one. Then did "direction" specific logic inside of that. Since the "if(leftReleased )" and "if(rightReleased)" both share the same code you can just combine them into one if statement by using an "or" operator.

EDIT: Sorry about editing this 5 times :)

SeraphLance

2,613

January 23, 2015 04:19 AM

In this case you could remove a lot of that code. Here's my refactor of it:
if(state == ActionState.RUNNING) {
    if(direction == Direction.LEFT) {
        runLeftAnim.update();
    } else if(direction == Direction.RIGHT) {
        runRightAnim.update();
    }

    if(leftReleased || rightReleased) {
        state = ActionState.IDLE;
    }
}
As you can see I combined the two top level "if(state == ActionState.Running)" statements into one. Then did "direction" specific logic inside of that. Since the "if(leftReleased )" and "if(rightReleased)" both share the same code you can just combine them into one if statement by using an "or" operator.

EDIT: Sorry about editing this 5 times

In the greater scheme of things, that might be too much refactoring. The idea is to not break clear intent. It's hard to say whether your refactor does that in this small example, but it could be in a larger program. Just saying this as food for thought.

Hodgman

52,717

January 23, 2015 04:28 AM

You can probably remove the duplication with something like:

if(state == ActionState.RUNNING)
{
	ASSERT( direction == Direction.LEFT || direction == Direction.RIGHT );
	runAnim[direction].update();
	if(runKeys[direction] == RELEASED)
		state = ActionState.IDLE;
}

. 22 Racing Series .

phil_t

8,087

January 23, 2015 04:46 AM

It's hard to offer refactoring suggestions without seeing how your code is structured overall (Maybe you should have a notion of the "current animation", which is selected from a table/array based on the state and direction. Then there is code that just calls update() on whatever the "current animation" is. Transition to ActionState.IDLE based on runKeys is done elsewhere, as it has nothing specifically to do with updating an animation).

But from what you provided, something like what Hodgman suggested seems right. The more stuff you can move into tables/arrays/data -> the less conditional logic you have to write -> the fewer bugs you have a chance to write.

Cascade Quest - IceFall Games - Dev blog - twitter - youtube

alvaro

21,604

January 23, 2015 01:27 PM

From what I can gathered about duplicate code, it is if the list of code that occurs more than once. Okay, based on what context? Is a duplicate code based on the similar appearance of the code?

Sure, similar appearance is how you normally identify opportunities to remove duplicate code. But the key test for me is, if I ever need to modify one of the blocks, am I likely to need to modify the other blocks in the same way? If that's the case, it's probably worth refactoring. Hodgman's suggestion is precisely how I think this should be done: Repetitive code structures can usually be refactored so the code is compact and the different cases are handled by a data structure.

Nicholas Kong

1,535

Author

January 23, 2015 10:46 PM

But the key test for me is, if I ever need to modify one of the blocks, am I likely to need to modify the other blocks in the same way

oh yeah. I never questioned that. But I always have the thought of it. That is a great way to identify duplicate code.

Say, is producing duplicate code off the start a bad habit? I find myself doing it a lot.

ApochPiQ

23,138

January 23, 2015 10:57 PM

Producing bad code is a bad habit, but not the end of the world.

Producing bad code and failing to clean it up as soon as possible is a much worse habit.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

jbadams

26,386

January 24, 2015 10:07 AM

You probably just need practice -- it's not always immediately clear how you'll be able to generalise your ideas to produce less duplicated code, but as long as you continue working on it you'll get better with practice. Look for duplicated code in your projects and make the effort to clean it up into a more generalized form.

- Jason Astle-Adams

Would this be considered duplicate code?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Would this be considered duplicate code?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines