1
\$\begingroup\$

Coming to the close of week 1 learning a programming language (Python)

Looking for general review, comments, and tips

Objective of Code
The objective here was to practice pulling information from dictionaries within lists and also adding dictionaries created from user input.

Code

def make_album(artist, album, tracks =''):
    catalog = {}
    catalog['artist_name'] = artist
    catalog['album_title'] = album
    catalog['tracks'] = tracks

    return catalog

def ask_user(message):
    user_input =''
    while not user_input:
        user_input = input(message)
    return user_input

riven = make_album('Zeppelin', 'Houses of the Holy', 8)
jax = make_album('Tool', 'Lateralus', 13)
vayne = make_album('Pink Floyd', 'Dark Side of the Moon')

differents = [riven, jax, vayne]

while True:
    print("type q to quit")
    band = ask_user("Enter artist: ")
    if band == "q":
        break
    album = ask_user("Enter album: ")
    if album == 'q':
        break
    numbers = input("Enter number of tracks: ")
    if numbers == 'q':
        break
    fire = make_album(band.title(), album.title(), numbers)
    differents.append(fire) 


for i in differents:
    if i['tracks']:
        print('\n' + i['album_title'] + ' by ' \
            + i['artist_name'] + ' it has '\
            + str(i['tracks']) +' tracks.')
    else:
        print('\n' +  i['album_title'] + ' by ' + i['artist_name'] \
            + '.')

Output

vash@localhost:~/pcc/8$ python3 lot2learn.py
type q to quit
Enter artist: circa survive
Enter album: juturna
Enter number of tracks: 11
type q to quit
Enter artist: dance gavin dance
Enter album: happiness
Enter number of tracks:
type q to quit
Enter artist: q

Houses of the Holy by Zeppelin it has 8 tracks.

Lateralus by Tool it has 13 tracks.

Dark Side of the Moon by Pink Floyd.

Juturna by Circa Survive it has 11 tracks.

Happiness by Dance Gavin Dance .
(xenial)vash@localhost:~/pcc/8$
\$\endgroup\$
  • \$\begingroup\$ @Graipher when you have time! \$\endgroup\$ – vash_the_stampede Aug 24 '18 at 12:53
1
\$\begingroup\$
  1. You should put all code that isn't in a function in a if __name__ == '__main__' guard. So that it doesn't run when it shouldn't.
  2. You can simplify quitting by using exceptions. If you make a function that raises say KeyboardInterupt if the input is 'q' then you can reduce the amount of code needed.
  3. I like to not use variables when they're not needed, or increase readability, and so you may want to move riven straight into the creation of differents.
  4. You can use f-strings or str.format to build your prints for you:

    >>> i = {'title': 'title', 'album': 'album'}
    >>> i['title'] + ' by ' + i['album']
    'title by album'
    >>> "{i[title]} by {i[album]}".format(i=i)
    'title by album'
    # Python 3.6+
    >>> f"{i['title']} by {i['album']}"
    'title by album'
    
  5. You can simplify make_album by using dictionary sugar:

    def make_album(artist, album, tracks =''):
        return {
            'artist_name': artist,
            'album_title': album,
            'tracks': tracks
        }
    
  6. You can alturnatly use collections.namedtuple:

    # Python 3.7
    Album = collections.namedtuple('Album', 'artist album tracks', defaults=('',))
    
    # Otherwise
    Album = collections.namedtuple('Album', 'artist album tracks')
    
    
    def make_album(artist, album, tracks=''):
        return Album(artist, album, tracks)
    
  7. I'd create a couple more functions. But honestly your code is pretty great. Half my suggestions I wouldn't expect a beginner to use.

import collections

Album = collections.namedtuple('Album', 'artist album tracks', defaults=('',))


def ask_user(message):
    user_input = None
    while not user_input:
        user_input = input(message)
    return user_input


def check_quit(user_input):
    if user_input == 'q':
        raise KeyboardInterupt()
    return user_input


def get_additional_albums():
    albums = []
    try:
        print("type q to quit")
        while True:
            albums.append(Album(
                cheack_quit(ask_user("Enter artist: ")).title(),
                cheack_quit(ask_user("Enter album: ")).title(),
                cheack_quit(input("Enter number of tracks: "))
            ))
    except KeyboardInterupt:
        pass
    return albums


if __name__ == '__main__':
    differents = [
        Album('Zeppelin', 'Houses of the Holy', 8),
        Album('Tool', 'Lateralus', 13),
        Album('Pink Floyd', 'Dark Side of the Moon')
    ]
    differents += get_additional_albums()

    for i in differents:
        output = f'{i.title} by {i.album}'
        if i.tracks:
            output = f'{output} it has {i.tracks} tracks'
        print(f'\n{output}.')
\$\endgroup\$
  • \$\begingroup\$ Thank you! All this is so exciting, every time you suggested new ways to write code that I never seen, or new commands, my eyes light up ha it gives me something new to learn, understand, and integrate, I want to write super clean and simplified code. All this is greatly appreciated I will not let it go to waste. That dictionary sugar you mentioned omg thats sick, using that everytime. \$\endgroup\$ – vash_the_stampede Aug 24 '18 at 15:05
  • \$\begingroup\$ unsure how to format comments with newlines but would you suggest something like this for the differents differents =[] differents.append(make_album('Zeppelin', 'Houses of the Holy', 8)) differents.append(make_album('Tool', 'Lateralus', 13)) differents.append(make_album('Pink Floyd', 'Dark Side of the Moon')) \$\endgroup\$ – vash_the_stampede Aug 24 '18 at 15:17
  • \$\begingroup\$ @vash_the_stampede That's good to hear! :) I'd not use that, as you have to type differents a lot, and it doesn't read as nice \$\endgroup\$ – Peilonrayz Aug 24 '18 at 15:18
  • \$\begingroup\$ hmm for now I suppose I could write a function that appends to differents and then call that function for now, would be cleaner, I just want to elimate those variables as you said, but going to learn your way and then apply that ! \$\endgroup\$ – vash_the_stampede Aug 24 '18 at 15:20
  • \$\begingroup\$ @vash_the_stampede You could just use differents = [a, b, c] such as in my answer (and your question) :) \$\endgroup\$ – Peilonrayz Aug 24 '18 at 15:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.