grim7reaper a écritLe layout du code est un peu bizarre par endroit je trouve, mais bon c’est ton style je suppose ^^
Tu as le droit de dire que c'est moche ^^' C'est grosso-modo ce qui se passe quand j'essaye d'appliquer sérieusement PEP8 à un moment ou à un autre: je finis par craquer et me mettre à faire n'importe quoi juste pour m'amuser. Le code de Touhy en fait les frais aussi.
grim7reaper a écritFonction `query`
Tu pourrais utiliser le context manager pour avoir
auto/commit-rollback
Cf.
la doc.
Je vais creuser ça, oui 🙂 Je ne suis pas familier avec ce genre de syntaxe, mais ça a effectivement l'air préférable.
grim7reaper a écritFonction `check`
Tu pourrais pas utiliser finally pour le conn.close?
Effectivement. Je n'ai pas assez ce réflexe (j'ai mis du temps à découvrir le finally, et je ne l'ai encore jamais utilisé en Python)
grim7reaper a écritFonction `escape`
Je ne comprends pas trop tes critères d’échappements.
J’ai trouvé
ce post à ce sujet, mais ça ne correspond peut-être pas à ce que tu veux faire.
J'avais la flemme de vérifier quels caractères on pouvait, ou pas, utiliser pour les noms de tables et de colonnes dans SQLite, alors j'ai fais un truc simple et dont j'étais sûr qu'il marcheravit à tous les coups: uniquement des lettres non-accentuées, des chiffres et des
underscore, en s'assurant que le nom de la table commence par une lettre (ce qui me permet d'utiliser des noms commençant par un
underscore pour les tables/colonnes utilisées par l'appli pour fonctionner sans risquer de conflit).
(À la base, c'est une version légèrement modifée du code qui me sert à échapper les mots pour mon jeu de pendu)
grim7reaper a écritFonction `loaddate`
Cette ligne est pas franchement claire :
return tuple(int(part) for part in parts)+(1,)*(3-len(parts))+(len(parts),)
Le retour doit être de la forme «(année, mois, jours, précision)», avec la précision qui varie entre 1 (à l'année près) et 3 (au jour près), en fonction de ce que contient la chaîne d'entrée («2010-05», par exemple, correspond à Mai 2010 avec une précision de 2, et le jour étant inconnu, on va mettre le premier du mois).
Du coup, un split sur le tiret, dont on convertit les éléments en entiers, on bourre des 1 à la fin, et on ajoute la taille pour la précision.
grim7reaper a écritIl pourrait aussi y avoir une docstring sur le format géré en entrée.
En effet. Le code entier manque cruellement de commentaire. C'est moche, mais ça m'évitait d'avoir à me décider entre le français (c'est un petit machin fait sur mesure, même pas internationalisé) et l'anglais (qui fait un brin plus sérieux pour un dépôt ouvert et avec des technos pas obsolètes pour une fois).
grim7reaper a écritFonction `savedate`
Les 0,1,2, et 3 pourrait être une énumération pour plus de lisibilité.
Sans doute, mais je n'ai encore jamais créé d'énumérations en Python, il faudra que j'aille me renseigner sur comment on fait.
grim7reaper a écritFonction `loadcoords`
Il pourrait y avoir une docstring sur le format géré en entrée. Ce n’est pas clair en lisant le code.
Y’a une trailing space l. 131
Chez moi, l'espace en trop est ligne 128 :o Mais merci, je vais corriger.
Pour le format d'entrée, c'est vrai que ce n'est pas bien clair. D'ailleurs, j'en ai changé en cours de route. Tu sais s'il existe un format «standard» facile à parser pour noter les lattitude/longitude? (Du genre du «AAAA-MM-JJ» pour les dates).
grim7reaper a écritFonction `savecoords`
Il pourrait y avoir une docstring sur le format produit en sortie.
Tu pourrais lancer une exception un peu plus claire que Exception (qui est très générique), et puis ajouter un message d’erreur aussi.
Le «Exception» tout court vient du fait que le code était initialement intégré à la fonction qui se sert de ça (dans _data.py), à l'intérieur d'un try/catch, que j'ai déplacé pour tenter vaguement de clarifier un brin. C'est vrai que, maintenant, ça manque de précision.
Il faudrait de toute façon que je reprenne la découpe des différentes parties un peu plus sérieusement.
grim7reaper a écritDans le "main"
C’est bizarre ce code au milieu de nulle part, ni dans une fonction ni guardé par un
if __name__ == '__main__':
if dbfile is None: sys.exit(0)
=> Pourrait donner un message d’erreur.
Il n'y a pas de conditionnelle, parce que ce code n'est jamais en position de __main__ (mais c'est vrai que, du coup, ça fait très bizarre).
Au lancement du programme, le fichier principal (elzbase.py) importe le fichier _base.py, qui gère toute la connexion à la base. Comme c'est ce fichier qui gère ça, il est le seul à avoir besoin de connaître le fichier contenant la base. Donc, il ouvre sys.argv, regarde s'il y a un nom de fichier précisé dedans, et, s'il n'y en a pas, ouvre une popup de sélection de fichier.
Le seul cas pour lequel dbfile peut valoir None, donc, c'est quand l'utilisateur a fermé cette popup sans avoir sélectionné de fichier, ce que j'interprète comme une demande de fermeture de l'appli (donc une fermeture normale, exit(0) et pas de message d'erreur). Mais c'est vrai que ça mériterait, au minimum, d'être commenté.
grim7reaper a écritelif os.access(dbfile, os.R_OK) and os.access(dbfile, os.W_OK):
=> Fait plutôt un
try/catch (
os.access ne vérifie pas que c’est un fichier, donc tu peux toujours échouer).
Hum, os.access renvoie False quand le fichier demandé n'existe pas, quel que soit le droit d'accès demandé. Ce qui correspond à ce que je veux à ce niveau (même si, cf la discussion précédente, ça n'assure pas qu'il n'y a pas de changements sur le disque entre temps).
D'ailleurs, tu peux constater toi-même qu'il y a bien un try/catch juste en dessous, qui gère la même chose (même si son rôle à l'origine est de s'assurer que le fichier qui va être ouvert est bien une base et contient bien les bonnes tables minimales). En revanche, la création de la variable tables est un brin mal placée.
Mais ouaip, y a sans doute de la clarification à faire à ce niveau aussi.
grim7reaper a écritC’est tout pour le moment 😃
C'est déjà pas mal 😃
D'ailleurs, si ça intéresse d'autres gens, coder une petite appli de ce genre peut former un genre de défi TdC, ça fait longtemps ^^
