Skip to content
Snippets Groups Projects
Commit f3ade224 authored by Théo LUDWIG's avatar Théo LUDWIG
Browse files

Merge branch 'fix' into 'main'

fix: better code to reverse string

See merge request !1
parents 92a3a10a 356db1c6
1 merge request!1fix: better code to reverse string
......@@ -9,3 +9,74 @@ Il est demandé à ce qu'une chaîne de caractères de longueur 10 au plus soit
```sh
gcc -Wall -Wextra -Werror main.c -o main.out
```
## Revue de code
### Constantes et éviter les valeurs "magiques"
Le code refactorisé utilise `MAXIMUM_STRING_LENGTH` pour définir la longueur maximale d'une chaîne plutôt que d'utiliser une valeur "magique" (`10` dans ce cas).
On utilise également `EXIT_SUCCESS` et `EXIT_FAILURE` pour définir les codes de sortie du programme, plutôt que d'utiliser des valeurs "magiques" (`0` et `1` dans ce cas).
### Nommage des variables
Le code refactorisé utilise des noms de variables/fonctions plus explicites, comme `string_reverse` plutôt que `my_function`, `temporary` plutôt que `temp`, `string_reversed` plutôt que `buffer` pour le résultat de la chaîne inversée, `index` plutôt que `i` pour l'index de la boucle, etc.
Permet de comprendre plus facilement le rôle de chaque variable/fonction, vaut mieux éviter l'implicite, les acronymes, et abréviations, et préférer des noms explicites qui ne laissent pas de place au doute.
### Séparation des Responsabilités
Le rôle du `main` est de récupérer l'input d'un utilisateur, d'appeler la fonction `string_reverse` et d'afficher le résultat.
Ce n'est pas à la fonction `string_reverse` d'afficher le résultat.
En séparant les responsabilités, on peut réutiliser la fonction `string_reverse` dans d'autres programmes qui n'ont pas besoin d'afficher le résultat sur `stdout`.
### Responsabilité de l'allocation mémoire et Performances
Copier une variable, peut être coûteux en termes de performances, et n'est pas nécessaire dans tous les cas, en + de doubler la consommation mémoire.
Dans notre cas, on a juste besoin d'afficher la chaîne de caractères originale/d'entrée, puis la chaîne de caractères inversée. On n'a pas besoin de garder en mémoire la chaîne de caractères originale, une fois celle-ci affichée, donc on peut économiser l'allocation `malloc` et la copie `strcpy` en combinant 2 `printf`, un pour la chaîne de caractères originale, et un pour la chaîne de caractères inversée.
Si plus tard, on a besoin de garder en mémoire la chaîne de caractères originale, on pourra toujours faire une copie dans le `main` sans modifier la fonction `string_reverse`.
```c
char* string_reversed = malloc(sizeof(char) * string_input_length + 1);
strcpy(string_reversed, string_input);
string_reverse(string_reversed);
// plus tard..., ne pas oublier de libérer la mémoire
free(string_reversed);
```
**=> Les copies des variables pris en argument dans les fonctions sont à éviter, le + souvent possible, sauf si strictement nécessaire. Afin de laisser le choix à l'appelant de garder ou non la variable originale en mémoire, en faisant une copie lui-même si il le souhaite.**
### Séparer affichage `stdout` et `stderr`
L'affichage sur `stdout` est réservé aux résultats, et l'affichage sur `stderr` est réservé aux erreurs.
```c
fprintf(stderr, "The string has more than %d characters.\n", MAXIMUM_STRING_LENGTH);
```
### Documentation
La documentation est importante, elle permet de comprendre le rôle de chaque fonction, et de savoir comment les utiliser.
```c
/**
* @brief Reverse the characters in a string.
*
* NOTE: Mutates the string.
*
* @param string
*/
```
### Tests Automatisés
Pour l'ajout de toute fonction/utilitaire, il faudrait rajouter des tests unitaires, afin d'éviter les régressions, et de s'assurer que le code fonctionne comme prévu.
Le mieux serait de le faire dans un fichier à part, qui contient uniquement les tests, et qui n'est pas compilé dans le programme final, de "production".
Ici il faudrait tester la fonction `string_reverse`.
......@@ -2,22 +2,44 @@
#include <stdlib.h>
#include <string.h>
void my_function(char* input) {
char* buffer = malloc(10);
strcpy(buffer, input);
for (size_t i = 0; i < sizeof(buffer) / 2; i++) {
char tmp = buffer[i];
buffer[i] = buffer[sizeof(buffer) - 1 - i];
buffer[sizeof(buffer) - 1 - i] = tmp;
#define MAXIMUM_STRING_LENGTH 10
/**
* @brief Reverse the characters in a string.
*
* NOTE: Mutates the string.
*
* @param string
*/
void string_reverse(char* string) {
size_t string_length = strlen(string);
size_t index_start = 0;
size_t index_end = string_length - 1;
while (index_start < index_end) {
char temporary = string[index_start];
string[index_start] = string[index_end];
string[index_end] = temporary;
index_start++;
index_end--;
}
printf("Reverse string of %s is %s\n", input, buffer);
}
int main(int argc, char** argv) {
if (argc < 2) {
printf("Usage: %s <input string>\n", argv[0]);
exit(1);
fprintf(stderr, "Usage: %s <string_input>\n", argv[0]);
return EXIT_FAILURE;
}
char* string_input = argv[1];
size_t string_input_length = strlen(string_input);
if (string_input_length > MAXIMUM_STRING_LENGTH) {
fprintf(stderr, "The string has more than %d characters.\n", MAXIMUM_STRING_LENGTH);
return EXIT_FAILURE;
}
my_function(argv[1]);
return 0;
printf("Reverse string of \"%s\" is", string_input);
string_reverse(string_input);
printf(" \"%s\".\n", string_input);
return EXIT_SUCCESS;
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment