Amélioration d'un code de débutant

Résolu/Fermé
spoiledog - Modifié le 19 mai 2021 à 16:34
mamiemando Messages postés 33079 Date d'inscription jeudi 12 mai 2005 Statut Modérateur Dernière intervention 23 avril 2024 - 30 mai 2021 à 22:33
Bonjour,
Pouvez-vous me proposer des améliorations pour ce code s'il vous plaît !
Merci infiniment.

print("\033[94m Outil de gestion de musique par listes de dictionnaires")

# Création:

compositeurs = [
    {'comp': 'Mozart', 'a_naiss': 1756, 'a_mort': 1791},
    {'comp': 'Beethoven', 'a_naiss': 1770, 'a_mort': 1827},
    {'comp': 'Haendel', 'a_naiss': 1685, 'a_mort': 1759},
    {'comp': 'Schubert', 'a_naiss': 1797, 'a_mort': 1828},
    {'comp': 'Vivaldi', 'a_naiss': 1678, 'a_mort': 1741},
    {'comp': 'Monteverdi', 'a_naiss': 1567, 'a_mort': 1643},
    {'comp': 'Chopin', 'a_naiss': 1810, 'a_mort': 1849},
    {'comp': 'Bach', 'a_naiss': 1685, 'a_mort': 1750}
]
oeuvres = [
    {'comp': 'Vivaldi', 'titre': 'Les quatres saisons', 'duree': 20, 'interpr': 'T. Pinnock'},
    {'comp': 'Mozart', 'titre': 'Concerto piano N°12', 'duree': 25, 'interpr': 'M. Perahia'},
    {'comp': 'Brahms', 'titre': 'Concerto violon N°2', 'duree': 40, 'interpr': 'A. Grumiaux'},
    {'comp': 'Beethoven', 'titre': 'Sonate "au clair de lune"', 'duree': 14, 'interpr': 'W. Kempf'},
    {'comp': 'Beethoven', 'titre': 'Sonate "pathétique"', 'duree': 17, 'interpr': 'W. Kempf'},
    {'comp': 'Schubert', 'titre': 'Quinquette "la truite"', 'duree': 39, 'interpr': 'SE of London'},
    {'comp': 'Haydn', 'titre': 'La création', 'duree': 109, 'interpr': 'H. von Karajan'},
    {'comp': 'Chopin', 'titre': 'Concerto pinao N°1', 'duree': 42, 'interpr': 'M.J Pires'},
    {'comp': 'Bach', 'titre': 'Toccata & fugue', 'duree': 9, 'interpr': 'P. Burmester'},
    {'comp': 'Beethoven', 'titre': 'Concerto piano N°4', 'duree': 33, 'interpr': 'M. Pollini'},
    {'comp': 'Mozart', 'titre': 'Symphonie N°40', 'duree': 39, 'interpr': 'F. Bruggen'},
    {'comp': 'Mozart', 'titre': 'Concerto piano N°22', 'duree': 35, 'interpr': 'S. Richter'},
    {'comp': 'Beethoven', 'titre': 'Concerto piano N°23', 'duree': 37, 'interpr': 'S. Richter'}
]

# Fonction 1:


def find_oeuvres(sub):  # Trouve les oeuvres qui ont pour titre un bout de la recherche
    resultat = []  # On stockera ici les résultats
    for oeuvre in oeuvres:  # On boucle sur les oeuvres
        if sub in oeuvre['titre']:
            resultat.append(oeuvre)  # Quand on trouve une oeuvre qui match le titre on l'ajoute à resultat
    print(resultat)

# Fonction 2:


def comp_infos(pieces):  # On cherche les infos de chaque compositeur d'une liste de pieces (d'oeuvres)
    for piece in pieces:  # On Boucle sur les oeuvres
        comp = piece['comp']  # On récupère le compositeur de notre oeuvre
        for compositeur in compositeurs:
            # On boucle sur chaque compositeur de la liste pour trouver les info de celui qui nous intéresse
            a_naiss, a_mort = '????', '????'  # Par défaut on dit que a_naiss et a_mort valent ????
            if compositeur['comp'] == comp:
                # Si on trouve le compositeur dans la liste alors on rempli a_naiss et a_mort
                a_naiss = compositeur['a_naiss']
                a_mort = compositeur['a_mort']
                break  # On arrête une fois le compositeur trouvé
        print(piece['titre'], piece['comp'], a_naiss, a_mort)

# Fonction 3:


def comps_alive(year):
    comps_vivant = []  # On stockera ici la liste des compositeurs vivant
    for compositeur in compositeurs:  # On boucle sur tous les compositeurs
        if (compositeur['a_naiss'] <= year) and (compositeur['a_mort'] >= year):  # Si cette condition est remplie
            comps_vivant.append(compositeur['comp'])  # On ajoute le compositeur à comps_vivant
    print("les compositeurs vivants en cette année: ")
    print(comps_vivant)


# Fonction 4:

def longest_piece_author():
    durees = []  # On stockera les durees des oeuvres ici
    for oeuvre in oeuvres:  # On boucle sur les oeuvres pour récupérer les durées
        durees.append(oeuvre['duree'])  # On ajoute chaque durée à la liste durees
    duree_max = max(durees)  # On récupère le max de ces durées
    for oeuvre in oeuvres:
        # On boucle encore une fois sur chaque oeuvre pour trouver celle qui a la durée qui vaut duree_max
        if oeuvre['duree'] == duree_max:
            print(oeuvre['comp'])  # On affiche le compositeur de l'oeuvre en question


# Test des Fonctions:

try:
    print("Question 2:\033[0m ")
    frag = input("Donner le fragment recherché: ")
    find_oeuvres(frag)  # Appel pour tester (question 2)
    print("\n\033[96m Question 3:\033[0m ")
    test_piece = [
        {'comp': 'Vivaldi', 'titre': 'Les quatres saisons', 'duree': 20, 'interpr': 'T. Pinnock'},
        {'comp': 'Mozart', 'titre': 'Concerto piano N°12', 'duree': 25, 'interpr': 'M. Perahia'},
        {'comp': 'Brahms', 'titre': 'Concerto violon N°2', 'duree': 40, 'interpr': 'A. Grumiaux'},
        {'comp': 'Beethoven', 'titre': 'Sonate "au clair de lune"', 'duree': 14, 'interpr': 'W. Kempf'},
        {'comp': 'Beethoven', 'titre': 'Sonate "pathétique"', 'duree': 17, 'interpr': 'W. Kempf'},
        {'comp': 'Schubert', 'titre': 'Quinquette "la truite"', 'duree': 39, 'interpr': 'SE of London'},
        {'comp': 'Haydn', 'titre': 'La création', 'duree': 109, 'interpr': 'H. von Karajan'},
        {'comp': 'Chopin', 'titre': 'Concerto pinao N°1', 'duree': 42, 'interpr': 'M.J Pires'},
        {'comp': 'Bach', 'titre': 'Toccata & fugue', 'duree': 9, 'interpr': 'P. Burmester'},
        {'comp': 'Beethoven', 'titre': 'Concerto piano N°4', 'duree': 33, 'interpr': 'M. Pollini'},
        {'comp': 'Mozart', 'titre': 'Symphonie N°40', 'duree': 39, 'interpr': 'F. Bruggen'},
        {'comp': 'Mozart', 'titre': 'Concerto piano N°22', 'duree': 35, 'interpr': 'S. Richter'},
        {'comp': 'Beethoven', 'titre': 'Concerto piano N°23', 'duree': 37, 'interpr': 'S. Richter'}
    ]
    # j'ai pris toute la liste pour raffiner le test
    comp_infos(test_piece)  # Appel pour tester (question 3)
    print("\n\033[96m Question 4:\033[0m ")
    y = int(input("Donner l'année: "))
    comps_alive(y)  # Appel pour tester (question 4)
    print("\n\033[96m Question 5:\033[0m ")
    print("Le compositeur de la pièce la plus longue est: ")
    longest_piece_author()  # Appel pour tester (question 5)
except ValueError:
    print("Vérifiez vos saisies!!")
A voir également:

7 réponses

mamiemando Messages postés 33079 Date d'inscription jeudi 12 mai 2005 Statut Modérateur Dernière intervention 23 avril 2024 7 749
Modifié le 19 mai 2021 à 17:03
Bonjour,

Le programme est déjà bien écrit comme tel, même si on peut effectivement y apporter quelques améliorations :
  • Tu devrais éviter de mélanger des mots anglais et français (e.g. find_oeuvre)
  • Plutôt que d'utiliser
    "????"
    tu peux utiliser
    None
    .
  • Certains fonctions s'écrivent de manière plus concise e.g. avec des comprehension-list.
  • Généralement on découple l'affichage (donc dans ton cas, des
    print
    ) et les fonctions qui explore les données (donc tes
    print
    devraient être a l'extérieur de ces fonctions)
  • La séquence d'échappement
    print("\033[94m Outil  ...")
    est spécifique aux terminaux Linux, donc pas forcément recommandée.
  • Le programme devrait quitter avec par exemple
    sys.exit(1)
    quand l'exception ValueError est levée. Cela correspond au code d'erreur. Si le code d'erreur vaut 0 (comportement par défaut), c'est que le programme s'est bien déroulé. Sinon le code d'erreur est documenté et selon sa valeur, peut permettre au programme appelant d'adapter son comportement. Cela permet de détecter au niveau de l'appel de ton programme python que celui-ci a planté.


Exemple :

def find_oeuvres(sub):  # Trouve les oeuvres qui ont pour titre un bout de la recherche
    resultat = []  # On stockera ici les résultats
    for oeuvre in oeuvres:  # On boucle sur les oeuvres
        if sub in oeuvre['titre']:
            resultat.append(oeuvre)  # Quand on trouve une oeuvre qui match le titre on l'ajoute à resultat
    print(resultat)


... devient :

def find_oeuvres(sub):  # Trouve les oeuvres qui ont pour titre un bout de la recherche
    return [
        oeuvre
        for oeuvre in oeuvres  # On boucle sur les oeuvres
        if sub in oeuvre['titre']
    ]


Tu peux aussi utiliser la fonction
filter
.

def find_oeuvres(sub):  # Trouve les oeuvres qui ont pour titre un bout de la recherche
    return filter(
        lambda oeuvre: sub in oeuvre['titre'],
        oeuvres
    )


Bonne chance
1
yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024 1 476
19 mai 2021 à 16:42
bonjour
la fonction 4 me semble particulièrement mal conçue.
réfléchis à la manière dont tu chercherais la réponse dans la vie réelle.
par exemple si tu avais une bibliothèque avec 50 livres, et tu dois trouver l'auteur du livre ayant le plus de pages.
0
Phil_1857 Messages postés 1883 Date d'inscription lundi 23 mars 2020 Statut Membre Dernière intervention 28 février 2024 178
Modifié le 20 mai 2021 à 11:18
Bonjour,

Ah je vois que tu n'as pas adopté la structure que je te proposais dans ton appel précédent (problème de boucle for) ...

et la façon plus précise d'afficher une erreur dans except

D'ailleurs, tu devrais fermer l'autre appel
0
oui je l'ai changé carrément et pour le try je n'ai pas compris comment le faire alors je l'ai laissé
0

Vous n’avez pas trouvé la réponse que vous recherchez ?

Posez votre question
Phil_1857 Messages postés 1883 Date d'inscription lundi 23 mars 2020 Statut Membre Dernière intervention 28 février 2024 178
Modifié le 20 mai 2021 à 12:51
Ben, c'est tout simple, c'est comme je te l'ai montré:

en début de code, tu écris:

import sys


et au lieu de

print("Vérifiez vos saisies!!")


tu écris

print(sys.exc_info()[1])


N'oublie pas de marquer l'appel précédent comme résolu
0
Comment le marquer comme résolu ?? xD
0
yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024 1 476 > spoiledog
20 mai 2021 à 22:11
pas toujours simple si tu n'es pas inscrit sur le forum.
je l'ai fait.
0
spoiledog > yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024
20 mai 2021 à 22:57
tu as marqué cet appel comme résolu, on parlait du précédant (Problème de boucle for) :(
0
yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024 1 476 > spoiledog
20 mai 2021 à 23:12
corrigé.
0
merci :)
0
mamiemando Messages postés 33079 Date d'inscription jeudi 12 mai 2005 Statut Modérateur Dernière intervention 23 avril 2024 7 749
Modifié le 21 mai 2021 à 15:27
Juste pour revenir à la remarque sur de la fonction liée à la question 4.

1)a) Au début tu cherches la durée max. Il n'y a pas besoin de mémoriser toutes les durées pour maintenir au cours du parcours la durée max trouvé jusque là. Ainsi te pourrais te passer de la liste durees et économiser de la mémoire. On parle de footprint (empreinte) mémoire, et si n désigne le nombre d'oeuvre, il est ici en O(n) (il croit linéairement avec le nombre d'oeuvre).

Algorithmiquement cela revient à modifier ta boucle comme suit :

duree_max = None
for oeuvre in oeuvres:
  duree = oeuvre["duree"]
  if duree_max is None or duree > duree_max:
    duree_max = duree


Tu as alors un footprint en O(1).

Mais on peut faire plus élégant grâce :
  • à la fonction
    max()
    en python. ;
  • au concept de générateurs (c'est typiquement ce que retourne la fonction
    filter
    )


Ainsi le bloc précédent s'écrit tout simplement :

duree_max = max(oeuvre["duree"] for oeuvre in oeuvres)


1)b) L'autre aspect pour concevoir un algorithme est sa complexité. Ici tu as deux boucles
for
sur les oeuvres (chacune en O(n)), donc l'algorithme fonctionne en O(n). Passer à la fonction
max
ne change rien de ce côté, car sa complexité est aussi O(n).

2) La seconde partie est un filtrage (qui mélange aussi la partie affichage, qui devrait être réalisé à l'extérieur de la fonction pour dissocier la manipulation des données et leur présentation). Je ne reviens pas trop sur comment faire le filtrage de manière plus élégante, j'en ai déjà parlé dans mon message précédent. Pour voir l'intérêt de cette remarque, il faut t'imaginer que ton programme pourrait un jour être utilisé dans une interface graphique. Dans ce cas là, la fonction de recherche reste la même, mais le print devrait être remplacé par ce qu'il faut pour corriger ton interface graphique. C'est le point de départ de ce qu'on appelle en architecture logicielle le modèle MVC.

Ainsi ta fonction
longest_pieces_author()
(pieces, comme il y en a plusieurs) deviendrait plutôt, si on renomme tes variables globales en majuscules, conformément aux recommandations PEP-8 :

def longest_pieces_author(oeuvres):
  duree_max = max(oeuvre['duree'] for oeuvre in oeuvres)
  return [
    oeuvre
    for oeuvre in oeuvres
    if oeuvre['duree'] == duree_max
  ]

from pprint import pprint
pprint(longest_pieces_author(OEUVRES))


Bonne chance
0
yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024 1 476
Modifié le 21 mai 2021 à 16:22
Tout cela ne change rien au défaut principal de l'algorithme proposé: la présence de la seconde boucle, révoltante d'inefficacité.
Pourquoi travailler en deux passes alors qu'une suffit?
def longest_pieces_author(oeuvres):
    duree_max = None
    for oeuvre in oeuvres:
      duree = oeuvre["duree"]
      if duree_max is None or duree > duree_max:
        duree_max = duree
        auteur = oeuvre['comp']
    return auteur
0
yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024 1 476 > yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024
21 mai 2021 à 16:30
ou bien, si on veut récupérer plusieurs auteurs:
def longest_pieces_author(oeuvres):
    duree_max = None
    for oeuvre in oeuvres:
      duree = oeuvre["duree"]
      if duree_max is None or duree > duree_max:
        duree_max = duree
        auteurs= []
        auteurs.append(oeuvre['comp'])
      elif duree == duree_max
        auteurs.append(oeuvre['comp'])
    return auteurs
0
mamiemando Messages postés 33079 Date d'inscription jeudi 12 mai 2005 Statut Modérateur Dernière intervention 23 avril 2024 7 749 > yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024
Modifié le 28 mai 2021 à 22:15
Hé bien... ma version ne doit pas être si sale que ça :-) Je te laisse tester sur ton PC.

from random import randint

OEUVRES = [
    {
        "title" : "title%s" % i,
        "duration"  : randint(0, 10)
    } for i in range(100000)
]

def longest_pieces_author_mando(oeuvres):
    duree_max = max(oeuvre['duration'] for oeuvre in oeuvres)
    return [
        oeuvre
        for oeuvre in oeuvres
        if oeuvre['duration'] == duree_max
    ]

def longest_pieces_author_ygbe(oeuvres):
    duree_max = None
    for oeuvre in oeuvres:
        duree = oeuvre["duration"]
        if duree_max is None or duree > duree_max:
            duree_max = duree
            auteurs = [oeuvre]
        elif duree == duree_max:
            auteurs.append(oeuvre)
    return auteurs


Ensuite si tu testes dans un jupyter notebook :

%time
x = longest_pieces_author_mando(OEUVRES)
CPU times: user 2 µs, sys: 0 ns, total: 2 µs
Wall time: 3.34 µs

%time
x = longest_pieces_author_ygbe(OEUVRES)
CPU times: user 8 µs, sys: 0 ns, total: 8 µs
Wall time: 15.7 µs


Ça peut surprendre, d'autant que ma version est significativement plus rapide (facteur 5). L'explication est double :
  • la complexité en temps de
    longest_pieces_author_mando
    et
    longest_pieces_author_ygbe
    est dans les deux cas O(n), donc ta proposition n'est pas plus rapide sur le plan théorique ;
  • en terme d'implémentation
    max
    va très vite ; le fait d'utiliser des générateurs et comprehension-list plutôt que des boucles for comme tu l'as fait permet à python d'exécuter le code plus rapidement.


Mais je comprends que tu aies tiqué, tu as sans doute fait tes armes sur du C ou un langage dans le genre, où ta remarque aurait sans doute été justifiée (pas de fonction max + léger gain de temps à faire une boucle for au lieu de deux -- en tout cas pas un facteur 2).
0
yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024 1 476 > mamiemando Messages postés 33079 Date d'inscription jeudi 12 mai 2005 Statut Modérateur Dernière intervention 23 avril 2024
21 mai 2021 à 17:32
C'est uniquement une question de principe.
Python est souvent utilisé pour apprendre les concepts de programmation.
Dans ce contexte, je pense préférable d'éviter tout ce qui est trop spécifique à Python, cela distrait des concepts à apprendre. Et compliquera le passage à un autre langage.
Surtout pour des débutants, il me semble important de concevoir un algorithme efficace, puis de le transcrire dans un langage.
Celui qui ne se rend pas compte qu'une passe suffit utilisera toujours deux passes, même si les données ne sont pas en memoire, ou trop nombreuses que pour être mémorisées simultanément.
0
mamiemando Messages postés 33079 Date d'inscription jeudi 12 mai 2005 Statut Modérateur Dernière intervention 23 avril 2024 7 749 > yg_be Messages postés 22720 Date d'inscription lundi 9 juin 2008 Statut Contributeur Dernière intervention 23 avril 2024
Modifié le 30 mai 2021 à 22:37
Je suis d'accord sur le fait que savoir factoriser du code est important et qu'il faut savoir le voir.

Ensuite par rapport au débat de départ :
  • Soit on parle de complexité théorique (en mémoire ou temps) et faire une boucle ou deux revient au même (par exemple entre un tri rapide et un tri à bulle, il n'y a pas d'hésitation). Par rapport à ta dernière remarque, qui en gros parle de l'empreinte mémoire, tu peux constater que les deux solutions sont équivalentes.
  • Soit on parle de performance observée et dans ce cas, la solution que je propose est plus rapide, plus "modulaire", et plus lisible. Dans ce cas, il faut garder en tête qu'en python, une boucle classique coûte plus cher qu'une comprehension-list ou qu'une built-in.


Ensuite le problème c'est aussi la justesse du code. Dans la fonction que tu proposes je viens de m'apercevoir d'un problème : la fonction ne marchera que si la liste d'œuvre est non vide. C'est dû à une erreur de design dans ta fonction : la liste d'auteurs devrait être (quel que soit le langage) être déclarée en dehors de la boucle
for
.
0