Skip to main content
added 1 character in body
Source Link
Reinderien
  • 65.4k
  • 5
  • 69
  • 188

Don't repeat yourself

This:

CLIENT_SECRETS_FILES = [
    'client_secret-1.json',
    'client_secret-2.json',
    'client_secret-3.json',
    'client_secret-4.json',
    'client_secret-5.json',
    ]
CREDENTIALS_FILES = [
    'credentials-1.json',
    'credentials-2.json',
    'credentials-3.json',
    'credentials-4.json',
    'credentials-5.json',
    ]

can be

CLIENT_SECRET_FILES = [f'client_secret-{i}' for i in range(1, 6)]
CREDENTIAL_FILES = [f'credentials-{i}' for i in range(1, 6)]

Also note that you shouldn't double-pluralize your variable names.

Empty comments

For this:

""" """

Fill it out, or delete it.

Generator mutability

This:

    players = list(chain(Goalie.objects.all(), Skater.objects.all()))
    for player in tqdm(players):

should not use list, because you never change the contents. Use tuple instead.

Just break

This:

            updated = False
            while CREDENTIALS_FILES and not updated:
                link = get_link(player.name)
                if link:
                    player.relevant_video = link
                    player.video_link_updated_at = datetime.datetime.now()
                    player.save(update_fields=['relevant_video', 'video_link_updated_at'])
                    updated = True

should not use a flag to break out of the loop. Instead, just issue break.

Do the simpler thing first

This:

if player.relevant_video:
    return((datetime.date.today() - player.video_link_updated_at).days > LINK_UPDATE_TIME)
else:
    return True

is more legible as:

if not player.relevant_video:
    return True

diff = datetime.date.today() - player.video_link_updated_at
return diff.days > LINK_UPDATE_TIME

Positive logic

This:

def credentials_not_valid(credentials):

can lend itself to confusing logic by the caller. Try to avoid negative predicates, and instead make this a positive predicate (credentials_valid).

Don't repeat yourself

This:

CLIENT_SECRETS_FILES = [
    'client_secret-1.json',
    'client_secret-2.json',
    'client_secret-3.json',
    'client_secret-4.json',
    'client_secret-5.json',
    ]
CREDENTIALS_FILES = [
    'credentials-1.json',
    'credentials-2.json',
    'credentials-3.json',
    'credentials-4.json',
    'credentials-5.json',
    ]

can be

CLIENT_SECRET_FILES = [f'client_secret{i}' for i in range(1, 6)]
CREDENTIAL_FILES = [f'credentials-{i}' for i in range(1, 6)]

Also note that you shouldn't double-pluralize your variable names.

Empty comments

For this:

""" """

Fill it out, or delete it.

Generator mutability

This:

    players = list(chain(Goalie.objects.all(), Skater.objects.all()))
    for player in tqdm(players):

should not use list, because you never change the contents. Use tuple instead.

Just break

This:

            updated = False
            while CREDENTIALS_FILES and not updated:
                link = get_link(player.name)
                if link:
                    player.relevant_video = link
                    player.video_link_updated_at = datetime.datetime.now()
                    player.save(update_fields=['relevant_video', 'video_link_updated_at'])
                    updated = True

should not use a flag to break out of the loop. Instead, just issue break.

Do the simpler thing first

This:

if player.relevant_video:
    return((datetime.date.today() - player.video_link_updated_at).days > LINK_UPDATE_TIME)
else:
    return True

is more legible as:

if not player.relevant_video:
    return True

diff = datetime.date.today() - player.video_link_updated_at
return diff.days > LINK_UPDATE_TIME

Positive logic

This:

def credentials_not_valid(credentials):

can lend itself to confusing logic by the caller. Try to avoid negative predicates, and instead make this a positive predicate (credentials_valid).

Don't repeat yourself

This:

CLIENT_SECRETS_FILES = [
    'client_secret-1.json',
    'client_secret-2.json',
    'client_secret-3.json',
    'client_secret-4.json',
    'client_secret-5.json',
    ]
CREDENTIALS_FILES = [
    'credentials-1.json',
    'credentials-2.json',
    'credentials-3.json',
    'credentials-4.json',
    'credentials-5.json',
    ]

can be

CLIENT_SECRET_FILES = [f'client_secret-{i}' for i in range(1, 6)]
CREDENTIAL_FILES = [f'credentials-{i}' for i in range(1, 6)]

Also note that you shouldn't double-pluralize your variable names.

Empty comments

For this:

""" """

Fill it out, or delete it.

Generator mutability

This:

    players = list(chain(Goalie.objects.all(), Skater.objects.all()))
    for player in tqdm(players):

should not use list, because you never change the contents. Use tuple instead.

Just break

This:

            updated = False
            while CREDENTIALS_FILES and not updated:
                link = get_link(player.name)
                if link:
                    player.relevant_video = link
                    player.video_link_updated_at = datetime.datetime.now()
                    player.save(update_fields=['relevant_video', 'video_link_updated_at'])
                    updated = True

should not use a flag to break out of the loop. Instead, just issue break.

Do the simpler thing first

This:

if player.relevant_video:
    return((datetime.date.today() - player.video_link_updated_at).days > LINK_UPDATE_TIME)
else:
    return True

is more legible as:

if not player.relevant_video:
    return True

diff = datetime.date.today() - player.video_link_updated_at
return diff.days > LINK_UPDATE_TIME

Positive logic

This:

def credentials_not_valid(credentials):

can lend itself to confusing logic by the caller. Try to avoid negative predicates, and instead make this a positive predicate (credentials_valid).

Source Link
Reinderien
  • 65.4k
  • 5
  • 69
  • 188

Don't repeat yourself

This:

CLIENT_SECRETS_FILES = [
    'client_secret-1.json',
    'client_secret-2.json',
    'client_secret-3.json',
    'client_secret-4.json',
    'client_secret-5.json',
    ]
CREDENTIALS_FILES = [
    'credentials-1.json',
    'credentials-2.json',
    'credentials-3.json',
    'credentials-4.json',
    'credentials-5.json',
    ]

can be

CLIENT_SECRET_FILES = [f'client_secret{i}' for i in range(1, 6)]
CREDENTIAL_FILES = [f'credentials-{i}' for i in range(1, 6)]

Also note that you shouldn't double-pluralize your variable names.

Empty comments

For this:

""" """

Fill it out, or delete it.

Generator mutability

This:

    players = list(chain(Goalie.objects.all(), Skater.objects.all()))
    for player in tqdm(players):

should not use list, because you never change the contents. Use tuple instead.

Just break

This:

            updated = False
            while CREDENTIALS_FILES and not updated:
                link = get_link(player.name)
                if link:
                    player.relevant_video = link
                    player.video_link_updated_at = datetime.datetime.now()
                    player.save(update_fields=['relevant_video', 'video_link_updated_at'])
                    updated = True

should not use a flag to break out of the loop. Instead, just issue break.

Do the simpler thing first

This:

if player.relevant_video:
    return((datetime.date.today() - player.video_link_updated_at).days > LINK_UPDATE_TIME)
else:
    return True

is more legible as:

if not player.relevant_video:
    return True

diff = datetime.date.today() - player.video_link_updated_at
return diff.days > LINK_UPDATE_TIME

Positive logic

This:

def credentials_not_valid(credentials):

can lend itself to confusing logic by the caller. Try to avoid negative predicates, and instead make this a positive predicate (credentials_valid).