6
\$\begingroup\$

I currently working on CS50. There is a problem where we need to recreate half-pyramids using hashes (#). The hashes are to substitute blocks like in Mario.

This is also the first time I'm using Python. I don't really know Python. So I just try to calculate myself based on my experience from C language.

from sys import exit
from cs50 import get_int

def main():
    while True:
        num = get_int("Height: ")
        if num >= 1 and num <= 8:
            break

    print_pyramid(num)
    exit(0)

def print_pyramid(n):
    for i in range(n):
        print_spaces(i, n)              # calling two function that print spaces and hash
        print_hash(i)                   # after print every spaces and hash in each row,
        print()                         # print new line for new row

def print_spaces(i, n):
    for j in range(n-(i+1)):            # those calculation means that we print space one less than the height      ......#  (seven spaces)
        print(" ", end="")              # if height = 8     8-(i+1)     'i' is increment from above function        .....##  (six spaces)

def print_hash(i):
    for l in range(i+1):                # print the hash by increasing it   (i+1)
        print("#", end="")              # i = 0  # / i = 1  ##

    l = 1                               # there's two spaces between two pyramid. at the print_spaces function range(n - i+1)
    n = 4                               # so initialize l = 1 and n = 4, would always give (4-(1+1)) = 2
    print_spaces(l, n)

    for l in range(i+1):                # print rigth hash 'pyramid'
        print("#", end="")

main() 

I always love to work with complicated idea using a lot of function for their different purpose.

I also got a 98% score from CS50.

What do you think of my code? Since this is my first time using Python, are there any Pythonic ways to simplify my program? (especially using function like I did).

Height: 4
       #  #
      ##  ##
     ###  ###
    ####  ####
\$\endgroup\$
6
\$\begingroup\$

Your code is quite nice. Your use of functions seem quite reasonable. And your style is nice too. Good job!

  • Calling print is expensive. To improve performance we can build the strings without using the for loop. For example to build \$n-(i + 1)\$ spaces we can use:

    >>> n = 12; i = 1
    >>> " " * (n - (i + 1))
    '          '
    
  • To build the pyramid you can delegate the creation of the spaces to Python by using str.ljust and str.rjust. These functions allow an easy to use way to build the pyramid.

    >>> 'abc'.ljust(10)
    'abc       '
    >>> 'abc'.rjust(10)
    '       abc'
    
  • Using print_spaces(l, n) is harder to understand than print(" ").

  • Your use of comments are not great. Most of them can be understood by just reading the code.

    I only found one of your comments to help. Because it explains something that isn't particuarly clear in clear English.

    # there's two spaces between two pyramid. at the print_spaces function range(n - i+1)
    # so initialize l = 1 and n = 4, would always give (4-(1+1)) = 2
    

    However this is a sign that your code is complicated. Which if you fix will make your comment redundent.

If we put these together we can greatly simplify your code.

from sys import exit
from cs50 import get_int

def main():
    while True:
        num = get_int("Height: ")
        if num >= 1 and num <= 8:
            break

    print_pyramid(num)
    exit(0)

def print_pyramid(n):
    for i in range(n):
        hashes = "#" * (i + 1)
        print(hashes.rjust(n) + "  " + hashes)

main()

There are still some more improvements that can be made.

  • Using sys.exit(0) is abnormal. It is normally best to just let Python handle that for you.

  • We can add an if __name__ == "__main__" guard to prevent your code from running if you import it accidentally.

    This is because if you use import filename you can import this program by accident and we don't want to run the main function if that happens.

  • I personally am a fan of always using the < operator. It just feels natural to see numbers ordered from smallest to largest, left to right.

  • We can Pythonify the print in print_pyramid by using the Format Specification Mini-Language.

    There are three parts to this.

    1. By using a format string we can enter values where we want them in a formatted string.

      >>> a = 1; b = 2; c = 3
      >>> f"{b}:{a} -> {c}"
      '2:1 -> 3'
      >>> "{b}:{a} -> {c}"  # Not a format string
      '{b}:{a} -> {c}'
      >>> "{b}:{a} -> {c}".format(a=a, b=b, c=c)  # Old style format strings
      '2:1 -> 3'
      
    2. We can use the alignment option to align the values where we want them to.

      >>> a = 1; b = 2; c = 3
      >>> f"{b:>3}:{a:<3} -> {c:^3}"
      '  2:1   ->  3 '
      
    3. We can enter the width using the same {} syntax as the values.

      >>> a = 1; b = 2; c = 3; width = 3
      >>> f"{b:>{width}}:{a:<{width}} -> {c:^{width}}"
      '  2:1   ->  3 '
      
from sys import exit
from cs50 import get_int


def main():
    while True:
        num = get_int("Height: ")
        if 1 <= num and num <= 8:
            break
    print_pyramid(num)


def print_pyramid(n):
    for i in range(n):
        hashes = "#" * (i + 1)
        print(f"{hashes:>{n}}  {hashes}")


if __name__ == "__main__":
    main()
\$\endgroup\$
  • \$\begingroup\$ Thank you so much. Your code is so simple. I'll try to understand it more deeply. \$\endgroup\$ – Kevinkun Oct 1 '20 at 17:55
4
\$\begingroup\$
  • You use three different names for the same thing. You ask for Height, then call it num, then call it n. I'd just call the variables height, both to be consistent and to be meaningful. At least in main and print_pyramid, as there it really means height. In print_spaces it's perhaps better to keep calling the parameter n, as that function doesn't need to know about pyramids and heights.
  • num >= 1 and num <= 8 can be the nicer 1 <= num <= 8 (or rather 1 <= height <= 8).
  • Your actual output does not match the claimed/desired(?) output you showed. You do not have those extra spaces at the start of each line to fill up the left pyramid half to width 8. Which I guess you should, as that would match the <= 8 condition, so that seems intended.
  • You use i at three places, and every single time you add 1 to it. Probably clearer to do that in the range right away, i.e., for i in range(1, n+1): instead of for i in range(n):.
\$\endgroup\$

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.