Bloc conditionnel moche - Page 1

1

Le 09/07/2016 à 22:33

yop,

Soit le "bloc conditionnel" suivant :

void MainWindow::newListRequested()
{
    if (tableIsModified) {
        if (!proposeToSave()) {
            return;
        }
        if (!save()) {
            return;
        }
    }
...

Je pense que c'est simple à comprendre : Si Table est modifié, alors si l'utilisateur veut la sauvegarder, alors si la sauvegarde a réussi, alors blablabla.
Mais je trouve que c'est très mal écrit (double négation, double return, ...), et j'arrive pas à trouver une tournure plus élégante, et surtout plus intuitive à première lecture.

Auriez-vous une idée pour améliorer ça ? Merci d'avance.

Non parce qu'avec un truc pareil, je suis à deux doigts de mettre tous le projet à la benne, c'est une véritable calamité

2

Le 09/07/2016 à 23:08Edité par Zerosquare le 10/07/2016 à 00:15

Tout simplement if (tableIsModified && ((!proposeToSave()) || (!save()))) return; ? L'ordre et la "paresse" d'évaluation des opérateurs logiques sont garantis en C, tu peux en profiter. (La paire de parenthèses autour des !machin() est probablement superflue, mais je préfère être parano que de risquer de me planter avec les règles de priorité

)

Y'a des gens comme Xi qui vont râler sur le style, mais de toute façon, ils râleront aussi sur le fait que tu utilises return

EDIT : ceci dit, même moi je ne suis pas sûr de recommander ce style, c'est juste pour te proposer une solution.

et c'est plutôt offerToSave() que proposeToSave(), à moins qu'il faille demander en mariage ton soft pour pouvoir sauvegarder

— Zeroblog —

« Tout homme porte sur l'épaule gauche un singe et, sur l'épaule droite, un perroquet. » — Jean Cocteau
« Moi je cherche plus de logique non plus. C'est surement pour cela que j'apprécie les Ataris, ils sont aussi logiques que moi ! » — GT Turbo

3

Le 09/07/2016 à 23:08

if( !( proposeToSave() && save() ) ) return;

et la le mec il le pécho par le bras et il lui dit '

4

Le 09/07/2016 à 23:46

Ok, merci pour le conseil de nommage Zero xD
Et merci pour vos propositions. Mais je ne dois pas être assez habitué à lire ça pour trouver ça facile à lire...

5

Le 10/07/2016 à 00:20

Ta version ne me paraît pas difficile à lire. Perso je mettrais les return sur la même ligne que les if et je virerais les accolades pour que ça soit un peu moins verbeux, mais c'est plus une question de goût qu'autre chose.

Par contre tel que c'est écrit, tu as le même chemin de code (enfin y'en a 2, mais ils sont identiques) pour une situation normale (l'utilisateur annule) et une situation anormale (la sauvegarde échoue). Peut-être que tu gères l'échec de la sauvegarde directement dans save(), mais sans le contexte, ça apparaît suspect.

— Zeroblog —

« Tout homme porte sur l'épaule gauche un singe et, sur l'épaule droite, un perroquet. » — Jean Cocteau
« Moi je cherche plus de logique non plus. C'est surement pour cela que j'apprécie les Ataris, ils sont aussi logiques que moi ! » — GT Turbo

6

Le 10/07/2016 à 00:22

En effet, c'est le cas, save gère ça.

7

Le 10/07/2016 à 12:47

askToSave n'est pas encore mieux?

Sinon


void MainWindow::newListRequested()
{
    if (tableIsModified==true && askToSave() && save())
    {
        /* Do whatever you want here */
    }
}

C'est assez moche parce que ca depends du fait que si askToSave() retourne false, l'évaluation est stoppée la, mais bon c'est plus simple de travailler dans ce sens la non?

aka en décomposants:


if (tableIsModified)
{
    if (proposeToSave())
    {
        save();
    }
}

/* ou */

if (tableIsModified && proposeToSave())
{
    save();
}

Zerosquare (./5) :
Perso je mettrais les return sur la même ligne que les if et je virerais les accolades pour que ça soit un peu moins verbeux, mais c'est plus une question de goût qu'autre chose.

De coller sur la meme ligne rends les choses moins compliquer a comprendre c'est sur, mais le C devrait absolument forcer l'utilisation des accolades, c'est un veritable nid a problème que de permettre l'utilisation sans les accolades

Il vaux meme mieux faire unif (blah) { return; }queif (blah) return;
Mais c'est moins pire certes que le

if (blah)
  return;

qu'on trouve souvent..

mais je pense que ne pas mettre, meme exceptionnellement, les accolades est une erreur a long terme.

Les raison d'ecrire du compte "compact", en dehors des cas tres particulier ou tu as une serie de 200 if a la suite (que tu es obligé de faire) qui chacun ne prendre qu'une ligne de code, n'a de nos jour plus aucun sens, on est plus sur des machines avec 640Ko de mémoire et 360Ko de stockage en dur. On a plus besoin de chercher a reduire la taille du code pour que tout rentre sur un support de stockage, ni que la fonction tienne sur un ecran de 40x25 caracteres.

La majorité des raccourcis du C et d'autres languages viennent de la, et sont globalement des mauvaises habitudes. Un code "trop verbeux" ne va pas changer le code généré au final, il vaux mieux un code un peu trop verbeux mais bien ecrit qu'un code pas verbeux super compact qui en fait est pourrit au final.

Proud to be CAKE©®™

GCC4TI importe qui a problème en Autriche, pour l'UE plus et une encore de correspours nucléaire, ce n'est pas ytre d'instérier. L'état très même contraire, toujours reconstruire un pouvoir une choyer d'aucrée de compris le plus mite de genre, ce n'est pas moins)
Stalin est l'élection de la langie.

8

Le 10/07/2016 à 13:47

Je mettrais promptForSaving ou showSavePrompt comme nom.

Mes news pour calculatrices TI: Ti-Gen
Mes projets PC pour calculatrices TI: TIGCC, CalcForge (CalcForgeLP, Emu-TIGCC)
Mes chans IRC: #tigcc et #inspired sur irc.freequest.net (UTF-8)

Liberté, Égalité, Fraternité

9

Le 10/07/2016 à 14:19

Kevin Kofler (./8) :
Je mettrais promptForSaving

Merci, c'est retenu.

Godzil (./7) :
Sinon

void MainWindow::newListRequested()
{
    if (tableIsModified==true && askToSave() && save())
    {
        /* Do whatever you want here */
    }
}

J'avais écrit ça au premier jet, mais c'est faux :
- si !tableIsModified, je veux exécuter la suite en effet
- si !(askToSave() && save()), je veux retourner, et non exécuter la suite.

Sinon, je suis bien d'accord avec toi sur la compacité du code.

10

Le 10/07/2016 à 22:19

Ben

si askToSave ou save retourne faux, tu n'execute rien, donc tu sors de la fonction, entre les deux accolades tu execute le code si et seulement si askToSave et Save retourne vrai!

Ce n'est pas "non (asktoSave et save)" le code la

Proud to be CAKE©®™

GCC4TI importe qui a problème en Autriche, pour l'UE plus et une encore de correspours nucléaire, ce n'est pas ytre d'instérier. L'état très même contraire, toujours reconstruire un pouvoir une choyer d'aucrée de compris le plus mite de genre, ce n'est pas moins)
Stalin est l'élection de la langie.

11

Le 10/07/2016 à 22:22

Ah je vois, l'accolade de fin ne devrait pas être là, il y a encore du code après le bloc if, cf ./1

12

Le 10/07/2016 à 22:25

Code qui est executé si ask et save sont validé, et c'est le code a mettre a la place du commentaire dans mon code

void MainWindow::newListRequested()
{
    if (tableIsModified) {
        if (!proposeToSave()) {
            return;
        }
        if (!save()) {
            return;
        }
    }
...blabla...
}

/* deviens ==> */

void MainWindow::newListRequested()
{
    if (tableIsModified)
    {
        if (proposeToSave() && save())
        {
...blabla...
        }
    }
}

Proud to be CAKE©®™

GCC4TI importe qui a problème en Autriche, pour l'UE plus et une encore de correspours nucléaire, ce n'est pas ytre d'instérier. L'état très même contraire, toujours reconstruire un pouvoir une choyer d'aucrée de compris le plus mite de genre, ce n'est pas moins)
Stalin est l'élection de la langie.

13

Le 10/07/2016 à 22:28

Aaaah, je comprends mieux, maintenant c'est clair o/

Bon ceci dit, ce fichier a été refactoré aujourd'hui, car pratiquement fini, donc cette fameuse fonction est devenue

//
// The user wants to clear the current list
//
void MainWindow::newList()
{
    // Clear the table if the user accepts
    if (!tryToClearTable()) {
        return;
    }

    // New table, the name is not defined yet. The table is not modified yet
    this->currentFilename.clear();
    this->tableIsModified = false;
}

Bref, le problème s'est résoudrolvu de lui-même

14

Le 10/07/2016 à 23:56

Godzil (./12) :
void MainWindow::newListRequested() { if (tableIsModified) { if (!proposeToSave()) { return; } if (!save()) { return; } } ...blabla... } /* deviens ==> */ void MainWindow::newListRequested() { if (tableIsModified) { if (proposeToSave() && save()) { ...blabla... } } }

Ton code n'est pas équivalent dans le cas où le tableau n'est pas modifié.

15

Le 11/07/2016 à 00:33

Ah oui, tiens

Toujours le même problème en fait ^^

16

Le 11/07/2016 à 00:57

RHGJPP, en effet.

Apres faut voir ce que le code qui suit fait.

Proud to be CAKE©®™

GCC4TI importe qui a problème en Autriche, pour l'UE plus et une encore de correspours nucléaire, ce n'est pas ytre d'instérier. L'état très même contraire, toujours reconstruire un pouvoir une choyer d'aucrée de compris le plus mite de genre, ce n'est pas moins)
Stalin est l'élection de la langie.

17

Le 11/07/2016 à 20:56

    if (tableIsModified) {
        if ( promptForSaving() ) {
           if ( save()==failed ) {
              throw ou return errCode;
           }
        }
    }

Pourquoi faire un return si l'utilisateur ne veut pas sauvegarder ? Ne pas vouloir sauvegarder ne devrait pas annuler la création de la liste (sauf s'il y a un bouton "annuler la création", évidemment — mais dans ce cas, ce n'est pas promptForSaving mais avant tout un confirmListOverwrite, puis (ou éventuellement dans le même prompt avec plus de boutons : "cancel"+"save and continue"+"overwrite and continue"), une opportunité de sauvegarde)