L’autre jour, je suis tombé sur le ticket d’un programmeur qui montrait un bout de son code, le voici :

bool SIM_FoodMemory::IsBoringChoice(std::string foodname)
{
	//return true if we ate the same food more than once in the last 14 days
	int recentmatches = 0;
	iter it;
	for(it = Items.begin(); it != Items.end(); ++it)
	{
		SIM_FoodMemoryItem* pitem = *it;
		if(pitem->FoodName == foodname)
		{
			int diff = SIM_GetSim()->GetTurnCount() - pitem->TurnConsumed;
			if(diff 			{
				recentmatches++;
			}
		}
	}

	if(recentmatches > 1)
	{
		return true;
	}
	else
	{
		return false;
	}
}

Autant vous le dire tout de suite, ce bout de code ne me plait pas du tout. Si l’on fait une remarque sur ce genre de code, beaucoup de coders sortent leur carte anti-troll ; « les goûts et les couleurs, ça ne se discute pas » ainsi que l’autre carte souvent utilisée ; « pas grave, ce n’est pas ‘time critic’ ».

Mais est-ce que ces 2 cartes devraient tout autoriser ? Alors sous ces 2 prétextes, on peut faire tout et n’importe quoi ? Je suis désolé, mais je ne suis pas d’accord.

Prenons le cas de la classe string qui est passé en paramètre, on est loin du débat « est-ce que ++i est mieux que i++ » ! Même si la fonction n’est pas ‘time critic’, passer une classe string comme cela entraine non seulement un overhead de code mais surtout, des allocations/désallocations mémoire! Il faut allouer la classe sur la pile, appeler le ctor, allouer la mémoire pour la chaine de caractère, la copier, et, à la fin, la désalouer. En plus de prendre du temps CPU, ça fragmente la mémoire, ce qui peut avoir des effets réellement génants.

Maintenant le fait de ne pas utiliser de const ? Mettre un header SIM_ au lieu d’utiliser un namespace ? Ce code montre très clairement que c’est un ancien programmeur C et qu’il ne maitrise pas des concepts basiques du C++ et continue à utiliser ses anciennes habitudes du C.

Tout le monde fait des erreurs, tout le monde a tendance à faire comme il a toujours fait, c’est humain. Ce qui me dérange plus, c’est la réaction, plutôt que d’accepter qu’il y ait un problème et le corriger, le programmeur se réfugie derrière le “c’est pas grave, les gouts et les couleurs… c’est pas time critic…”. Comment évoluer, s’améliorer dans ces conditions ?

Pour finir, voilà comment, moi, j’aurais surement écrit ce bout de code (ce n’est pas du tout pour montrer la solution parfaite, mais juste pour jouer le jeu) :

using namespace std;
namespace SIM {
bool FoodMemory::isBoringChoice(const string &foodname) const
{
	int recentmatches = 0;
	for(iter it = Items.begin(); it != Items.end(); it++)
	{
		FoodMemoryItem *item = *it;
		if(item->FoodName == foodname)
		{
			int diff = sim()->turnCount() - item->TurnConsumed;
			if(diff 		}
	}

	return (recentmatches > 1);
}

J’ai clairement un style plus compact, j’ai fait exprès de ne pas changer l’algo sinon j’aurais fait directement un return dans le if pour casser la boucle ‘for’ le plus tôt possible.

Et comme je suis curieux, vous l’auriez écrit comment vous ?