Amélioration d'un code de débutant Résolu/Fermé

Question

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("
\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("
\033[96m Question 4:\033[0m ")
    y = int(input("Donner l'année: "))
    comps_alive(y)  # Appel pour tester (question 4)
    print("
\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!!")

yg_be · Answer

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.

mamiemando · Answer

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

Phil_1857 · Answer

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

spoiledog · Answer

oui je l'ai changé carrément et pour le try je n'ai pas compris comment le faire alors je l'ai laissé

Phil_1857 · Answer

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

spoiledog · Answer

merci :)

mamiemando · Answer

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

Amélioration d'un code de débutant

7 réponses

Discussions similaires