suizokukan / katal

select a bunch of files, create a catalogue of the selected files without duplicates and tag them
GNU General Public License v3.0
8 stars 1 forks source link

A more pythonic code ? #58

Open ytterbium opened 8 years ago

ytterbium commented 8 years ago

Hello,

I looked a little inside the code, and found its style quite unusual for python code.

First, some strange conventions :

("a long, "
"very, "
"long "
"line") 

Some ugly backslash could be removed.

And finally about the msg function : maybe the logging module can do job as good as this function, with more flexibility.

It is only some suggestions, and maybe those points were intended. But if you want, I could transform all the msg logic in order to use the logging module.

(P.S. Do you mind if we continue in french ?

suizokukan commented 8 years ago

Thanks for the feedback ! Please, go on in French if you want : I will translate all that's important to be read by other people.

You're right the msg() function but I need time to think about it.

Merci encore de ces remarques que je trouve très constructives. Qu'en pensez-vous / penses-tu ?

ytterbium commented 8 years ago

Le problème n'est pas le nom des méthodes, mais celui des paramètres. Par exemple, pourquoi def msg(_msg, _for_console=True, _for_logfile=True, _consolecolor=None): et pas def msg(msg, for_console=True, for_logfile=True, consolecolor=None): ? Les underscores sont pour des attributs privés, ce qui n'est pas très utiles dans une fonction.

Pour les \, je suis tout à fait d'accord pour limiter la taille des lignes, mais on peut en éliminer certains en mettant entre parenthèses les strings (par exemple quand elles font plus de trois lignes).

s = 'du texte, ' \
      'du texte, ' \
      'toujours du texte'

est équivalent à

s = ('du texte, ' 
      'du texte, ' 
      'toujours du texte')

La deuxième forme limite le risque d'oublier un \ en codant.

On peut aussi éliminer des \" en remplçant les guillemets par ' ou """. Par exemple msg(' o about to remove "{0}" ') à la place de msg(" o about to remove \"{0}\" " )

Pour la fonction msg, si tu veux je peux essayer de la remplacer et de faire un pull requests.

suizokukan commented 8 years ago
suizokukan commented 8 years ago

Bon, en fait j'ai simplement ajouté une branch dev : j'y ai inclus un premier commit qui supprime les caractères "\" aberrants.

suizokukan commented 8 years ago

Les _ inutiles ont été virés mais il me faut relire la documentation dans katal.py : trop de choses ont été modifiées. Merci de ces suggestions, très intéressantes.

suizokukan commented 8 years ago

La branch dev contient le code annoncé : plus de _ au début des arguments des fonctions : commit 3bbad3b17b8ebbe5ccbe1c1c89ea43674a4df63b .