Posté le 09/07/2016 à 22:33 Membre depuis le 18/06/2001, -26078 message
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. smile

Non parce qu'avec un truc pareil, je suis à deux doigts de mettre tous le projet à la benne, c'est une véritable calamité embarrassed
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 09/07/2016 à 23:08Edité par Zerosquare le 10/07/2016 à 00:15 Membre depuis le 27/04/2006, 60480 messages
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é cheeky)

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 embarrassed
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 grin
avatarZeroblog

« 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
Posté le 09/07/2016 à 23:08 Membre depuis le 15/07/2002, 4490 messages
if( !( proposeToSave() && save() ) ) return;
Posté le 09/07/2016 à 23:46 Membre depuis le 18/06/2001, -26078 message
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...
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 10/07/2016 à 00:20 Membre depuis le 27/04/2006, 60480 messages
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.
avatarZeroblog

« 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
Posté le 10/07/2016 à 00:22 Membre depuis le 18/06/2001, -26078 message
En effet, c'est le cas, save gère ça.
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 10/07/2016 à 12:47 Membre depuis le 30/06/2001, 71425 messages
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 un
if (blah) { return; }
que
if (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.
avatarProud to be CAKE©®™
The cake is a lie! - Love your weighted companion cube

->986-Studio's Wonder Project!<-
yN a cassé ma signature :o
Posté le 10/07/2016 à 13:47 Membre depuis le 10/06/2001, 40275 messages
Je mettrais promptForSaving ou showSavePrompt comme nom.
avatarMes news pour calculatrices TI: Ti-Gen (fr/en), MobiFiles (de)
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é
Posté le 10/07/2016 à 14:19 Membre depuis le 18/06/2001, -26078 message
Kevin Kofler (./8) :
Je mettrais promptForSaving
Merci, c'est retenu. smile
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. smile
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 10/07/2016 à 22:19 Membre depuis le 30/06/2001, 71425 messages
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
avatarProud to be CAKE©®™
The cake is a lie! - Love your weighted companion cube

->986-Studio's Wonder Project!<-
yN a cassé ma signature :o
Posté le 10/07/2016 à 22:22 Membre depuis le 18/06/2001, -26078 message
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
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 10/07/2016 à 22:25 Membre depuis le 30/06/2001, 71425 messages
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 smile

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

/* deviens ==> */

void MainWindow::newListRequested()
{
    if (tableIsModified)
    {
        if (proposeToSave() && save())
        {
...blabla...
        }
    }
}
avatarProud to be CAKE©®™
The cake is a lie! - Love your weighted companion cube

->986-Studio's Wonder Project!<-
yN a cassé ma signature :o
Posté le 10/07/2016 à 22:28 Membre depuis le 18/06/2001, -26078 message
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 tripo
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 10/07/2016 à 23:56 Membre depuis le 04/05/2005, 4440 messages
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é.
avatar
Posté le 11/07/2016 à 00:33 Membre depuis le 18/06/2001, -26078 message
Ah oui, tiens triso Toujours le même problème en fait ^^
avatar<<< Kernel Extremist©®™ >>>
Feel the power of (int16) !
Posté le 11/07/2016 à 00:57 Membre depuis le 30/06/2001, 71425 messages
RHGJPP, en effet.

Apres faut voir ce que le code qui suit fait.
avatarProud to be CAKE©®™
The cake is a lie! - Love your weighted companion cube

->986-Studio's Wonder Project!<-
yN a cassé ma signature :o
Posté le 11/07/2016 à 20:56 Membre depuis le 10/06/2001, 45115 messages
    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)