Le fil des codeurs

[Librekom]

J'ouvre ce fil pour partager vos infos, remarques, questions a propos du code informatique au sens large du terme.

Je commence avec ce que je viens de coder en python. Je voudrais votre feedback.
Je sais qu'il y aurai eut moyen de faire plus simple, mais j'en suis encore au debut de ma formation.

Le but est de calculer le nombre de jours entre deux dates (en se basant sur le calendrier gregorien, en tenant compte des annees bisextiles et sans remonter dans le temps.

 

Exemple:

Question : combien de jours entre le 1 Fevrier 2011 et le 2 Fevrier 2011
Reponse : 1

def isLeapYear(year):
    #Returns True if year is a leap year. Otherwise returns False
    if year % 400 == 0:
        return True
    if year % 100 == 0:
        return False
    if year % 4 == 0:
        return True
    return False

def DaysInMonth(year, month):
    #Returns the number of day in a month
    if month == 1 or month == 3 or month == 5 or month == 7 \
    or month == 8 or month == 10 or month == 12:
        return 31
    else:
        if month == 2:
            if isLeapYear(year):
                return 29
            else:
                return 28
        else:
            return 30
           
def nextDay(year, month, day):
    #Returns for a date the date of the next day
    if day < DaysInMonth(year, month):
        return year, month, day + 1
    else:
        if month < 12:
            return year, month + 1, 1
        else:
            return year + 1, 1, 1
            
        
def dateIsBefore(year1, month1, day1, year2, month2, day2):
    #Returns True if year1-month1-day1 is before year2-month2-day2. Otherwise, returns False.
    if year1 < year2:
        return True
    if year1 == year2:
        if month1 < month2:
            return True
        if month1 == month2:
            return day1 < day2
    return False        

def daysBetweenDates(year1, month1, day1, year2, month2, day2):
    #Returns the number of days between year1/month1/day1 and year2/month2/day2.
    #Assumes inputs are valid dates in Gregorian calendar.
    days = 0
    assert not dateIsBefore(year2, month2, day2, year1, month1, day1)
    while dateIsBefore(year1, month1, day1, year2, month2, day2):
        year1, month1, day1 = nextDay(year1, month1, day1)
        days += 1
    return days

def test():
    test_cases = [((2012,1,1,2012,2,28), 58), 
                  ((2012,1,1,2012,3,1), 60),
                  ((2011,6,30,2012,6,30), 366),
                  ((2011,1,1,2012,8,8), 585 ),
                  ((1900,1,1,1999,12,31), 36523)]
    
    for (args, answer) in test_cases:
        result = daysBetweenDates(*args)
        if result != answer:
            print "Test with data:", args, "failed"
        else:
            print "Test case passed!"

test()

[Noob]

Que veux tu dire par sans remonter dans le temps ?

Sinon ça me semble correct, tu pourrais aussi faire des sauts des 365/366 jours puis des sauts de 31/30/29/28 jours puis enfin juste une addition.

Puis plutôt que de faire un assert not, tu pourrais simplement intervertir les arguments après un if pour que ça compte tout de même la différence. En plus ça va échouer si la différence est de zéro.

Après je connais pas tes consignes, peut-être que c'est tout à fait normal.

[Rübezahl]

Je voudrais votre feedback.

En général pour ces algorithmes il y a déjà des fonctions/librairies existantes et très bien faites.

(Mais coder ça soi-même c'est très bien aussi hein)

[Rincevent]

Oui. Apprendre à faire c'est bien, mais à un moment, quand on peut avoir des primitives un peu puissantes...

SELECT DATEDIFF(day, @Debut, @Fin);

Remarque, je me suis déjà amusé à utiliser un calendrier maison (pour jouer avec des dates ouvrées, donc en omettant les samedis, dimanches, et jours fériés) : c'est le métier qui remplissait régulièrement une table de jours ouvrés (donc avec les trous qui vont bien) et je jointais la table sur elle-même. J'étais jeune et je découvrais SQL.

[Sloonz]

Pour la partie Python :

 

 if month == 1 or month == 3 or month == 5 or month == 7 \
    or month == 8 or month == 10 or month == 12:

=>

 

if month in (1,3,5,7,8,10,12):

def dateIsBefore(year1, month1, day1, year2, month2, day2):
    return (year1, month1, day1) < (year2, month2, day2)

Dans DaysInMonth (à renommer en daysInMonth)/nextDay, remplacer :

 

if x:
    return ...
else:
    foo()

par

 

if x:
    return ...
foo()

(je sais que beaucoup prônent l’inverse, mais en pratique dans du code plus compliqué quand les cascades de if sont en fait des préconditions pour un algo complexe, c’est très pénible d’avoir la partie principale de l’algo à une profondeur trop importante. Et ça fout la merde avec les gestionnaires de révision, où ajouter/supprimer une précondition conduit à un diff sur la totalité de l’algo alors que seule la structure des préconditions a bougé…)

[Sloonz]

Dans tes tests unitaires, tu ne teste pas le cas year1 < year2, month1 > month2, cas qui pourrait pêter si tu te décides à suivre la suggestion de noob (sauter des années) (et pareil pour day1 > day2) sans penser à cette possiblité. Tu ne testes pas non plus le cas year1 = year2.

C’est le problème des tests unitaires chez les débutants (et pas que les débutants en fait) : coder un nombre de tests unitaires en raison de la probabilité du scénario. L’intérêt du test unitaire n’est pas de couvrir le plus possible de scénarios, mais de couvrir le plus possible de code. Dans l’absolu un test unitaire supplémentaire n’est pertinent que s’il couvre une branche de code (enfin, un chemin) pas encore couverte.

[Librekom]

Que veux tu dire par sans remonter dans le temps ?

C'est ma maniere a mois de dire que la date 2 ne peut pas etre anterieure a la date 1

Sinon ça me semble correct, tu pourrais aussi faire des sauts des 365/366 jours puis des sauts de 31/30/29/28 jours puis enfin juste une addition.

Pas compris

Puis plutôt que de faire un assert not, tu pourrais simplement intervertir les arguments après un if pour que ça compte tout de même la différence. En plus ça va échouer si la différence est de zéro.

Après je connais pas tes consignes, peut-être que c'est tout à fait normal.

Je reposterais les consignes exacte ce soir, la je suis au boulot, mais je crois que le but etait de nous apprendre a utiliser "assert"

[Librekom]

En général pour ces algorithmes il y a déjà des fonctions/librairies existantes et très bien faites.

(Mais coder ça soi-même c'est très bien aussi hein)

Oui c'est sur mais le but est d'apprendre le code.

C'est pareil que l'arithmetique et la calculatrice. On a toujours besoins d'apprendre a compter, meme si on a un calculatrice.

[Librekom]

Oui. Apprendre à faire c'est bien, mais à un moment, quand on peut avoir des primitives un peu puissantes...

SELECT DATEDIFF(day, @Debut, @Fin);

Remarque, je me suis déjà amusé à utiliser un calendrier maison (pour jouer avec des dates ouvrées, donc en omettant les samedis, dimanches, et jours fériés) : c'est le métier qui remplissait régulièrement une table de jours ouvrés (donc avec les trous qui vont bien) et je jointais la table sur elle-même. J'étais jeune et je découvrais SQL.

Je n'en suis pas encore la, je debute, il y a 2 semaines le mot string ne m'evouquait qu'un sous-vetement sexy.

[Librekom]

Pour la partie Python :

 

 if month == 1 or month == 3 or month == 5 or month == 7 \
    or month == 8 or month == 10 or month == 12:

=>

 

if month in (1,3,5,7,8,10,12):

Ah oui, ca par contre j'ai deja appris, j'aurais du y pensser.

def dateIsBefore(year1, month1, day1, year2, month2, day2):
    return (year1, month1, day1) < (year2, month2, day2)

Dans DaysInMonth (à renommer en daysInMonth)/nextDay, remplacer :

Pourquoi le renommer "daysInMonth" ? C'est une convention? J'ai effectivement vu que on faisait comme ca, mais je n'ai pas encore recu d'explication. C'est juste un convention ou y a une raison technique derriere?

if x:
    return ...
else:
    foo()

par

 

if x:
    return ...
foo()

(je sais que beaucoup prônent l’inverse, mais en pratique dans du code plus compliqué quand les cascades de if sont en fait des préconditions pour un algo complexe, c’est très pénible d’avoir la partie principale de l’algo à une profondeur trop importante. Et ça fout la merde avec les gestionnaires de révision, où ajouter/supprimer une précondition conduit à un diff sur la totalité de l’algo alors que seule la structure des préconditions a bougé…)

Tu veux dire que je doit virer tout les "Else" ?

[Sloonz]

Pourquoi le renommer "daysInMonth" ? C'est une convention? J'ai effectivement vu que on faisait comme ca, mais je n'ai pas encore recu d'explication. C'est juste un convention ou y a une raison technique derriere?

Convention effectivement :

- rester cohérent avec toi-même

- en général, en python, un identifiant commençant en majuscule désigne une classe (toujours par convention) (à part quelques classes de base qui sont en minuscule, principalement pour des raisons historiques)

 

Tu veux dire que je doit virer tout les "Else" ?

Pas de le bannir généralement . L’idée est de limiter la profondeur d’indentation en éliminant tôt:

- les cas triviaux

- les préconditions

en utilisant la forme if-return sans else.

Dans ta situation tu n’as que quelques cas (3 dans nextDay, 4 dans daysInMonth), tous trivaux, donc la différence n’est pas flagrante, mais imagine un peu à quelle profondeur tu devrais aller avec une structure if-else, dans une fonction qui aurait 10 cas triviaux à séparer de l’algo principal (et avec un algo principal vachement plus complexe).

(c’est ce que tu as fait dans isLeapYear d’ailleurs )

[Noob]

C'est ma maniere a mois de dire que la date 2 ne peut pas etre anterieure a la date 1

Ok, dans ce cas si c'est demandé, c'est normal.

Pas compris

Plutôt que d'augmenter le compteur days de 1 tant que dateIsBefore retourne vrai, tu pourrais augmenter de 365 si il y a une différence d'année et que la différence est de plus que 365 jours. Puis une dois que la différence est de moins de 365 jours, tu ajoutes le nombre de jours par mois, puis enfin la différences de jours.

 

Je reposterais les consignes exacte ce soir, la je suis au boulot, mais je crois que le but etait de nous apprendre a utiliser "assert"

Ok, oui dans ce cas c'est cohérent avec le point 1.

 

Pour la partie Python :

 

 if month == 1 or month == 3 or month == 5 or month == 7 \
    or month == 8 or month == 10 or month == 12:

=>

 

if month in (1,3,5,7,8,10,12):

def dateIsBefore(year1, month1, day1, year2, month2, day2):
    return (year1, month1, day1) < (year2, month2, day2)

Dans DaysInMonth (à renommer en daysInMonth)/nextDay, remplacer :

 

if x:
    return ...
else:
    foo()

par

 

if x:
    return ...
foo()

(je sais que beaucoup prônent l’inverse, mais en pratique dans du code plus compliqué quand les cascades de if sont en fait des préconditions pour un algo complexe, c’est très pénible d’avoir la partie principale de l’algo à une profondeur trop importante. Et ça fout la merde avec les gestionnaires de révision, où ajouter/supprimer une précondition conduit à un diff sur la totalité de l’algo alors que seule la structure des préconditions a bougé…)

 

+1, et même de façon général on pourrait stocker le nombre de jours par mois dans un tableau.

daysInMonths = [31,28,31,30,31,30,31,31,30,31,30,31] et si isLeapYear, on ajoute 1 à 28.

Convention effectivement :

- rester cohérent avec toi-même

- en général, en python, un identifiant commençant en majuscule désigne une classe (toujours par convention) (à part quelques classes de base qui sont en minuscule, principalement pour des raisons historiques)

 

Pas de le bannir généralement . L’idée est de limiter la profondeur d’indentation en éliminant tôt:

- les cas triviaux

- les préconditions

en utilisant la forme if-return sans else.

Dans ta situation tu n’as que quelques cas (3 dans nextDay, 4 dans daysInMonth), tous trivaux, donc la différence n’est pas flagrante, mais imagine un peu à quelle profondeur tu devrais aller avec une structure if-else, dans une fonction qui aurait 10 cas triviaux à séparer de l’algo principal (et avec un algo principal vachement plus complexe).

(c’est ce que tu as fait dans isLeapYear d’ailleurs )

+1 aussi pour tout ça.

[Librekom]

Merci a vous

[Asthenik]

Par curiosité, tu avais des contraintes dans ton énoncé pour employer une méthode aussi "tatonneuse" ? Habituellement, on fait d'abord les sauts d'années, on ajoute ou on soustrait les jours des mois de différences, puis on ajuste aussi avec les jours dans le mois.

Et puis pourquoi ne pas valider les dates en input ? Pourquoi cette contrainte sur la chronologie des 2 dates (un résultat négatif est tout à fait pertinent) ?

Aussi, une remarque, tu emploies ta méthode dateIsBefore comme condition de ta boucle, alors que ce que tu fais en réalité, c'est de rechercher quand les dates sont égales. Il faut essayer d'employer des méthodes bien en rapport avec leur finalité.