This forum is read only and just serves as an archive. If you have any questions, please post them on github.com/phoboslab/impact

I keep hearing how a properly written function should have a single function.

Take this naughty function for example which is called each time a bullet entity collides with the player entity:

```takeDamage: function(damage) {
if(this.sounds.impact) {
this.sounds.impact.play();
}
if(this.god) {
return;
}
this.lastHitTimer.reset();
if(this.shields > 0 && this.shields - damage >= 0) {
// The shield can take ALL of the blow.
this.shields -= damage;
} else if(this.shields > 0 && this.shields - damage < 0) {
// The shield can take SOME of the blow.
var healthDamage = damage - this.shields;
this.shields = 0;
this.health = Math.max(this.health - healthDamage, 0);
} else if (this.shields <= 0) {
// The shield can take NONE of the blow.
this.health = Math.max(this.health - damage, 0);
}
if(this.health <= 0) this.noMoreHealth();
}
```

According to the one task per function principal. This function should be rewritten like this:

```takeDamage: function(damage) {
if(this.shields > 0 && this.shields - damage >= 0) {
// The shield can take ALL of the blow.
this.shields -= damage;
} else if(this.shields > 0 && this.shields - damage < 0) {
// The shield can take SOME of the blow.
var healthDamage = damage - this.shields;
this.shields = 0;
this.health = Math.max(this.health - healthDamage, 0);
} else if (this.shields <= 0) {
// The shield can take NONE of the blow.
this.health = Math.max(this.health - damage, 0);
}
}
```

Yes, this looks easier to read and manage.

But now I'm left with the question, where do I put all the rest of the code I took out?

Before upon collision with the player, a bullet entity would call `player.takeDamage(100);` and that was it.

So does this mean that the bullet entity is supposed to call the following now instead?:

```player.playHitSound();
if(!player.god) {
player.takeDamage();
player.lastHitTimer.reset();
}
```

It seems like at the very least this is violating some object oriented programming principals. I mean, should the bullet entity really be aware that `player` has variables such as `god`, and `lastHitTimer`? Probably not.

I've probably structured things fundamentally wrong, but I'm not sure how I should have come at this any differently.

So, have you guys been having much success with "short concise single purpose functions", and do you see an elegant way to improve upon this example?
A well written function should have a single responsibility. This is of course more of a guideline than anything, so don't think of it like a hard and fast rule.

Your last method looks clean, but should be part of the player's takeDamage function, not the bullet's. You're right - the bullet should be dumb. Its single responsibility is to call takeDamage (or perhaps a better named onHit or something) with whatever entity it hits. That's it. It's up to the entity to decide how to deal with that. Then your player's takeDamage function should be something along the lines of your final function, only with respect to its internal state:

```takeDamage: function(damage) {
this.playHitSound();
if(!this.canTakeDamage()) {
this.applyDamage(damage);
this.lastHitTimer.reset();  // although this should be part of applyDamage, perhaps