Dragon Ball Clean Code - Las funciones deben hacer una sola cosa
Nuevo capítulo del libro de Clean Code: Funciones
En Clean Code, Uncle Bob nos recomienda que las funciones hagan sólo una cosa.
Parece hasta obvio y podríamos pensar que lo aplicamos en nuestro día a día, pero… si la función tiene más de 10 líneas empezamos a correr el riesgo de que no hagan una sola cosa.
Cuidado, recordemos las funciones descriptivas, podríamos pensar que una función descriptiva hace muchas cosas, pero lo que hace es una que necesita varios pasos y que están guiados por dicha función. Volvemos al primer refactor que hizo el Dr Brief en el post de Funciones cortas
private void initGravitySystem() {
boolean isNewGravitySet = setNewGravity();
giveFeedbackWhileSettingGravity();
startEmergencyService();
setRoomAmbient();
startTrainingSet();
}
Podríamos decir que esta función hace 5 cosas, pero en realidad hace 1: invocar cada uno de los pasos necesarios para hacer Inicializar el Sistema Gravitacional.
Volvamos al algoritmo original que debía refactorizar el Dr.
Comienza la aventura!
(Recordemos a qué nos enfrentábamos)
public class GravityManager {
public void mainProcess() {
float planetGravity = 0.0;
float resultingGravity = 0.0;
int gravityMultiplier = 0;
PlanetDictionary dict = new PlanetDictionary();
PlanetGravityReader reader = new PlanetGravityReader();
Planet planet = dict.getCurrentPlanet();
planetGravity = reader.readGravityFromPlanet(planet.getId());
if (planetGravity > 0) {
NumPadInputter inputter = new NumPadInputter();
while (inputter.isReading()) {
gravityMultiplier = inputter.getInput();
}
resultingGravity = planetGravity * gravityMultiplier;
ScreenOutput.showMessage("New gravity value: " + resultingGravity);
} else {
ScreenOutput.showMessage("Zero gravity can not be multiplied");
}
GravitationalForceAccelerator gaf = new GravitationalForceAccelerator();
gaf.setAccelerateFrom(planetGravity);
gaf.setAccelerateTo(resultingGravity);
gaf.startSlowGravityAcceleration();
RawSoundWave sound = SoundOutput.DigitalSound.parseString("PI PO PI, PO PI PI PO PIIIII");
SoundOutput.play(sound);
ScreenOutput.showMessage("Increasing gravity...");
float first25percent = (resultingGravity - planetGravity) * 0.25f;
float halfpercent = first25percent + first25percent;
float threequarters = first25percent * 3;
float hundredpercent = resultingGravity;
float currentGravity = planetGravity;
while (currentGravity <= first25percent) {
sound = SoundOutput.DigitalSound.parseString("PI");
sound.setBassDbTo(sound.getDefaultBassDb() + 30);
SoundOutput.play(sound);
Thread.sleep(5000);
currentGravity = gaf.getCurrentGravity();
}
while (currentGravity <= halfpercent) {
sound = SoundOutput.DigitalSound.parseString("PI");
sound.setBassDbTo(sound.getDefaultBassDb() + 10);
SoundOutput.play(sound);
Thread.sleep(3000);
currentGravity = gaf.getCurrentGravity();
}
while (currentGravity <= threequarters) {
sound = SoundOutput.DigitalSound.parseString("PI");
sound.setTrebleDbTo(sound.getDefaultTrebleDb() + 10);
SoundOutput.play(sound);
Thread.sleep(2000);
currentGravity = gaf.getCurrentGravity();
}
while (currentGravity <= hundredpercent) {
sound = SoundOutput.DigitalSound.parseString("PI");
sound.setTrebleDbTo(sound.getDefaultTrebleDb() + 30);
SoundOutput.play(sound);
Thread.sleep(1000);
currentGravity = gaf.getCurrentGravity();
}
sound = SoundOutput.DigitalSound.parseString("PI PI PI");
sound.setBassDbTo(sound.getDefaultBassDb());
sound.setTrebleDbTo(sound.getDefaultTrebleDb());
SoundOutput.play(sound);
...
/* De esta parte me preocuparé más adelante */
}
}
El Dr. Brief se había dejado un comentario TODO
para no olvidar donde se quedó:
private void initGravitySystem() {
boolean isNewGravitySet = setNewGravity();
// TODO: Continue over here...
giveFeedbackWhileSettingGravity();
startEmergencyService();
setRoomAmbient();
startTrainingSet();
}
Toca montar la función giveFeedbackWhileSettingGravity()
, con el ánimo de que sus funciones hagan una sola cosa y que sólo tengan un nivel de abstracción cada una, sacó toda la parte de feedback visual y sonoro del sistema de la parte donde se crea el sistema de gravitacional para moverlas a esta función.
private void giveFeedbackWhileSettingGravity() {
try {
showTargetGravityOnScreen();
// TODO: Manage sound alerts
} catch (ZeroGravityException zge) {
Log.e(zge.getMessage());
}
}
Vio que la gravedad actual y objetivo serían valores que necesitaría a menudo y que calcularlos cada vez era pesado, para no meterse en un refactor más grande, optó por crear un par de propiedades de clase que le permitieran acceder a estos valores con facilidad.
public class GravityManager {
float planetGravity = 0.0f;
float targetGravity = 0.0f;
...
}
Y ajustó el donde se inicializaban estos valores.
private boolean setNewGravity() {
this.planetGravity = getCurrentPlanetGravity(),
this.targetGravity = getTargetGravity(planetGravity);
...
}
Esta función debería ser suficiente para controlar el output deseado por pantalla.
private void showTargetGravityOnScreen() throws ZeroGravityException {
if (this.targetGravity <= 0) {
ScreenOutput.showMessage("Zero gravity can not be multiplied");
throw new ZeroGravityException();
}
ScreenOutput.showMessage("New gravity value: " + this.targetGravity, delay: 10);
ScreenOutput.showMessage("Increasing gravity...");
}
La siguiente parte se veía más complicada, parece una manera muy enrevesada de hacer que el sistema pite cada vez más rápido y más agudo conforme la gravedad va aumentando…
Listo para refactorizar pondría foco en que cada función hiciera una sola cosa.
Como hay mucho código que explicar, iremos comentando algunas líneas a ignorar sobre las que volveremos luego.
private void playSoundFeedbackWhileGravityChanges() {
playInitSystemSound();
//playGravityChangingSound();
//playEndSystemSound();
}
private void playInitSystemSound() {
static String INIT_SOUND_STRING = "PI PO PI, PO PI PI PO PIIIII";
RawSoundWave sound = getSound(INIT_SOUND_STRING, 0, 0);
SoundOutput.play(sound);
}
private void getSound(String soundPattern, int trebleVariation, int bassVariation) {
RawSoundWave sound = SoundOutput.DigitalSound.parseString(soundPattern);
sound.setTrebleDbTo(sound.getDefaultTrebleDb() + trebleVariation);
sound.setBassDbTo(sound.getDefaultBassDb() + bassVariation);
}
Dos funciones de 3 líneas, simple, para hacer sonar el sonido que indica que el sistema gravitacional va a empezar a hacer cosas
private void playSoundFeedbackWhileGravityChanges() {
//playInitSystemSound();
//playGravityChangingSound();
playEndSystemSound();
}
private void playEndSystemSound() {
static String END_SOUND_STRING = "PI PI PI";
RawSoundWave sound = getSound(END_SOUND_STRING, 0, 0);
SoundOutput.play(sound);
}
Para quitarnos de encima “lo fácil”, vemos como en otras 3 líneas lanzamos el sonido de cierre del sistema.
Y ahora, a la madre del cordero:
private void playSoundFeedbackWhileGravityChanges() {
//playInitSystemSound();
playGravityChangingSound();
//playEndSystemSound();
}
private void playGravityChangingSound() {
RawSoundWave sound;
int delay = 0;
while (getCurrentGravity() < this.targetGravity) {
float percentId = getGravityChangeProgress();
sound = getSoundByPercentId(percentId);
delay = getDelayByPercentId(percentId);
playSoundWithDelay(sound, delay);
}
}
Mientras no llegamos a la gravedad objetivo, según el porcentaje de progreso hacia la gravedad objetivo, haremos sonar un tipo de sonido u otro.
Vamos al detalle:
private float getCurrentGravity() {
return GravitationalForceAccelerator.getInstance().getCurrentGravity();
}
private int getGravityChangeProgress() {
float percent = getGravityChangeProgressPercent();
int percentId = GRAVITY_AT_100_PERCENT;
if (percent < 25.0f) {
percentId = GRAVITY_BELOW_25_PERCENT;
} else if (percent >= 25.0f && percent < 50.0f) {
percentId = GRAVITY_BETWEEN_25_AND_50_PERCENT;
} else if (percent >= 50.0f && percent < 75.0f) {
percentId = GRAVITY_BETWEEN_50_AND_75_PERCENT;
} else if (percent >= 75.0f && percent < 100.0f) {
percentId = GRAVITY_BETWEEN_75_AND_100_PERCENT;
}
return percentId;
}
private float getGravityChangeProgressPercent() {
return getCurrentGravity() / this.targetGravity * 100
}
private RawSoundWave getSoundByPercentId(percentId) {
String soundString = "PI";
int trebleVariation = 0;
int bassVariation = 0;
switch (percentId) {
case GRAVITY_BELOW_25_PERCENT:
bassVariation = 30;
break;
case GRAVITY_BETWEEN_25_AND_50_PERCENT:
bassVariation = 10;
break;
case GRAVITY_BETWEEN_50_AND_75_PERCENT:
trebleVariation = 10;
break;
case GRAVITY_BETWEEN_75_AND_100_PERCENT:
trebleVariation = 30;
break;
}
return getSound(soundString, trebleVariation, bassVariation);
}
private int getDelayByPercentId(percentId) {
int delay = 0;
switch (percentId) {
case GRAVITY_BELOW_25_PERCENT:
delay = 5000;
break;
case GRAVITY_BETWEEN_25_AND_50_PERCENT:
delay = 3000;
break;
case GRAVITY_BETWEEN_50_AND_75_PERCENT:
delay = 2000;
break;
case GRAVITY_BETWEEN_75_AND_100_PERCENT:
delay = 1000;
break;
}
return delay;
}
private void playSoundWithDelay(RawSoundWave sound, int delay) {
SoundOutput.play(sound);
Thread.sleep(delay);
}
Las funciones getCurrentGravity
y getGravityChangeProgressPercent
procuran hacer más legible la API y evitarse poner líneas de cálculos dentro de algoritmos más complejos. playSoundWithDelay
es suficientemente descriptiva.
getGravityChangeProgress
hace más homogénea la gestión del punto de progreso en el que nos encontramos. Respecto a la implementación inicial que hacía hasta 4 tipos de cálculos distintos, al pasar el estado del progreso a una enumeración, tambié lo hace que sea más legible.
Finalmente getSoundByPercentId
y getDelayByPercentId
, gracias a la enumeración, permiten abstraer la parametrización del sonido con retraso y hacer más concisa la función que hace lo operativo playGravityChangingSound
.
Han salido muchas funciones y explicado aquí, secuencialmente, puede parecer más complejo. Recordemos que en un IDE la navegación por el código es mucho más amigable.
Lo interesante aquí es que hemos pasado a funciones muy pequeñas y que sólo hacen una cosa y eso permite que sólo con su firma: el nombre y parámetros de la función, sea suficiente para hacernos una idea bastante clara de qué está ocurriendo sin necesidad de bajar a cada detalle de implementación.
El Dr. Brief no está súper orgulloso de este refactor, le chirrían un poco algunos if else if ... else
o tanto switch case
. Hay prácticas para hacerlo aún más mantenible, pero se tendría que meter en movidas de OOP como polimorfismo u otras técnicas. Para su cometido que era reducir las responsablidades de cada función a 1, se siente más o menos cómodo.
Veamos como ha quedado todo junto y cerramos la aventura de hoy!
(Nótese que las funciones más largas son meras clasificadoras con switch case
que son muy verbosos)
public class GravityManager {
float planetGravity = 0.0f;
float targetGravity = 0.0f;
// 7 lines
private void initGravitySystem() {
boolean isNewGravitySet = setNewGravity();
if (isNewGravitySet) {
giveFeedbackWhileSettingGravity();
// TODO: Do later
//startEmergencyService();
//setRoomAmbient();
//startTrainingSet();
}
}
// 4 lines
private boolean setNewGravity() {
this.planetGravity = getCurrentPlanetGravity(),
this.targetGravity = getTargetGravity(planetGravity);
boolean isNewGravitySet = startGravityAcceleration(planetGravity, toGravity);
return isNewGravitySet;
}
// 6 lines
private void giveFeedbackWhileSettingGravity() {
try {
showTargetGravityOnScreen();
playSoundFeedbackWhileGravityChanges();
} catch (ZeroGravityException zge) {
Log.e(zge.getMessage());
}
}
// 6 lines
private void showTargetGravityOnScreen() throws ZeroGravityException {
if (this.targetGravity <= 0) {
ScreenOutput.showMessage("Zero gravity can not be multiplied");
throw new ZeroGravityException();
}
ScreenOutput.showMessage("New gravity value: " + this.targetGravity, delay: 10);
ScreenOutput.showMessage("Increasing gravity...");
}
// 3 lines
private void playSoundFeedbackWhileGravityChanges() {
playInitSystemSound();
playGravityChangingSound();
playEndSystemSound();
}
// 3 lines
private void playInitSystemSound() {
static String INIT_SOUND_STRING = "PI PO PI, PO PI PI PO PIIIII";
RawSoundWave sound = getSound(INIT_SOUND_STRING, 0, 0);
SoundOutput.play(sound);
}
// 3 lines
private void getSound(String soundPattern, int trebleVariation, int bassVariation) {
RawSoundWave sound = SoundOutput.DigitalSound.parseString(soundPattern);
sound.setTrebleDbTo(sound.getDefaultTrebleDb() + trebleVariation);
sound.setBassDbTo(sound.getDefaultBassDb() + bassVariation);
}
// 8 lines
private void playGravityChangingSound() {
RawSoundWave sound;
int delay = 0;
while (getCurrentGravity() < this.targetGravity) {
float percentId = getGravityChangeProgress();
sound = getSoundByPercentId(percentId);
delay = getDelayByPercentId(percentId);
playSoundWithDelay(sound, delay);
}
}
// 1 line
private float getCurrentGravity() {
return GravitationalForceAccelerator.getInstance().getCurrentGravity();
}
// 12 lines
private int getGravityChangeProgress() {
float percent = getGravityChangeProgressPercent();
int percentId = GRAVITY_AT_100_PERCENT;
if (percent < 25.0f) {
percentId = GRAVITY_BELOW_25_PERCENT;
} else if (percent >= 25.0f && percent < 50.0f) {
percentId = GRAVITY_BETWEEN_25_AND_50_PERCENT;
} else if (percent >= 50.0f && percent < 75.0f) {
percentId = GRAVITY_BETWEEN_50_AND_75_PERCENT;
} else if (percent >= 75.0f && percent < 100.0f) {
percentId = GRAVITY_BETWEEN_75_AND_100_PERCENT;
}
return percentId;
}
// 1 line
private float getGravityChangeProgressPercent() {
return getCurrentGravity() / this.targetGravity * 100
}
// 19 lines
private RawSoundWave getSoundByPercentId(percentId) {
String soundString = "PI";
int trebleVariation = 0;
int bassVariation = 0;
switch (percentId) {
case GRAVITY_BELOW_25_PERCENT:
bassVariation = 30;
break;
case GRAVITY_BETWEEN_25_AND_50_PERCENT:
bassVariation = 10;
break;
case GRAVITY_BETWEEN_50_AND_75_PERCENT:
trebleVariation = 10;
break;
case GRAVITY_BETWEEN_75_AND_100_PERCENT:
trebleVariation = 30;
break;
}
return getSound(soundString, trebleVariation, bassVariation);
}
// 16 lines
private int getDelayByPercentId(percentId) {
int delay = 0;
switch (percentId) {
case GRAVITY_BELOW_25_PERCENT:
delay = 5000;
break;
case GRAVITY_BETWEEN_25_AND_50_PERCENT:
delay = 3000;
break;
case GRAVITY_BETWEEN_50_AND_75_PERCENT:
delay = 2000;
break;
case GRAVITY_BETWEEN_75_AND_100_PERCENT:
delay = 1000;
break;
}
return delay;
}
// 2 lines
private void playSoundWithDelay(RawSoundWave sound, int delay) {
SoundOutput.play(sound);
Thread.sleep(delay);
}
Agradecimientos
Gracias a mis compañeros y compañeras de Basetis por el acceso a este libro y la flexibilidad para escribir este contenido que comparto con vosotras.