5
\$\begingroup\$

I hacked a way to print a christmas tree, to use it in the usual seasons greetings email, you know the drill.

Next Step is to integrate a multi lines of text in the border left side of the tree. For this i like to use an offset to the TREE['center'].

""" Seasons Greeting mail - ascii Christmas Tree """
import pprint
import shutil
import sys

# DEFAULT
TREE = {
    'minimalHeight': 10,
    'minimalWidth': 12,
    'height': 10,
    'width': 12,
    'center': 6,
    'calc_width': lambda x: x + 2,
    'calc_center': lambda x: int(x / 2),
    'calc_amount': lambda x: int(x / 2),
}

(TREE['maximalWidth'], TREE['maximalHeight']) = shutil.get_terminal_size()


# FUNKTIONS
def line_generic_branch(imageline, amount, left):
    """
    generates a branch line
    """
    line_default(imageline)
    if left:
        start_pos = TREE['center'] - TREE['calc_amount'](imageline)
        for num in range(amount):
            tree[imageline][start_pos + num] = '_'
        tree[imageline][start_pos] = '('
    else:
        end_pos = TREE['center'] + TREE['calc_amount'](imageline)
        for num in range(amount):
            tree[imageline][end_pos - num] = '_'
        tree[imageline][end_pos] = ')'


def line_default(imageline):
    tree[imageline][0] = '|'
    tree[imageline][-1] = '|'


def line_0():
    for pos in range(1, TREE['width'] - 1):
        tree[0][pos] = '_'


def line_1():
    line_default(1)


def line_2():
    line_default(2)
    tree[2][TREE['center']] = ')'


def line_branch(imageline, left):
    line_generic_branch(imageline, imageline, left)


def line_last_branch(imageline, left):
    line_generic_branch(imageline, TREE['calc_amount'](imageline), left)


def line_first_trunk(imageline):
    line_default(imageline)
    if imageline % 2:
        tree[imageline][TREE['center']] = '('
    elif not imageline % 2:
        tree[imageline][TREE['center']] = ')'


def line_bottom_trunk(imageline):
    line_default(imageline)
    if imageline % 2:
        tree[imageline][TREE['center'] - 1] = '('
        tree[imageline][TREE['center']] = '.'
    elif not imageline % 2:
        tree[imageline][TREE['center'] + 1] = ')'
        tree[imageline][TREE['center']] = '.'


def line_last(imageline):
    line_default(imageline)
    for pos in range(1, TREE['width'] - 1):
        tree[imageline][pos] = '_'


# INIT
tree = []
imageline = 0
if len(sys.argv) > 1:
    try:
        TREE['height'] = int(sys.argv[1])
    except TypeError:
        raise TypeError("Only Integers are allowed not {}".format(sys.argv[1]))
    if TREE['height'] < TREE['minimalHeight']:
        raise ValueError("given Height is to less. MIN {}".format(
            TREE['minimalHeight']))
    if TREE['height'] > TREE['maximalHeight']:
        raise ValueError("given Height is to much. MAX {}".format(
            TREE['maximalHeight']))
    TREE['width'] = TREE['calc_width'](TREE['height'])
    TREE['center'] = TREE['calc_center'](TREE['width'])

# MAIN
tree = [[' ' for x in range(TREE['width'])] for y in range(TREE['height'])]
"""lets Draw the lines"""
while imageline <= TREE['height'] - 4:
    try:
        globals()['line_' + str(imageline)]()
    except KeyError:
        left = False
        if imageline % 2:
            left = True
        if imageline < TREE['height'] - 4:
            line_branch(imageline, left)
        elif imageline == TREE['height'] - 4:
            line_last_branch(imageline, left)
    imageline += 1

## add footer
line_first_trunk(TREE['height'] - 3)
line_bottom_trunk(TREE['height'] - 2)
line_last(TREE['height'] - 1)

for nr, line in enumerate(tree):
    #    print("{0:2d}".format(nr), ''.join(line))
    print(''.join(line))

https://github.com/mbenecke/christmas_tree

It workes fine, so far. Here an Example:

Di Nov 05 11:08:36 (master)> python christmastree.py 15
 _______________
|               |
|       )       |
|      (__      |
|      ___)     |
|     (____     |
|     _____)    |
|    (______    |
|    _______)   |
|   (________   |
|   _________)  |
|  (____        |
|       )       |
|      (.       |
|_______________|

But i am not a skilled coder, so i need someone who give me feedback to my Style. What I want reviewed:

  • Am I reinventing the wheel? I searched thoroughly but couldn't find anything similar.
  • Code style.
  • Is it pythonic?
  • Is it readable?
  • Hopefully not over engineered, but slim enough to be proud of it.
\$\endgroup\$
4
  • \$\begingroup\$ Hi welcome to code review. Please paste the actual code into the body of your question. \$\endgroup\$ Commented Nov 5, 2019 at 12:01
  • \$\begingroup\$ done, but as already statet in the question, i asume it is too large. \$\endgroup\$
    – Circusgimp
    Commented Nov 5, 2019 at 12:05
  • 2
    \$\begingroup\$ 130 lines of code are not really considered as excessive/"to much" here on Code Review. \$\endgroup\$
    – AlexV
    Commented Nov 5, 2019 at 12:35
  • 1
    \$\begingroup\$ length isn't an issue unless what you really want is Code Golf, like this christmas tree. But your code looks perfectly suitable for Code Review. \$\endgroup\$
    – Gloweye
    Commented Nov 5, 2019 at 12:39

1 Answer 1

7
\$\begingroup\$

Out of your defaults, only the minimal height have some relevance. The others can be deduced from it or are fixed computations that can be baked into the logic of a function.

I will also strongly suggest to use argparse for command-line arguments manipulation instead of manually trying to perform the conversion and range-checking. It is a bit overkill for a single argument here, but as soon as you will add more it will become way more readable and maintainable.


Your various branch construction logic is scattered all over the place and would benefit for a generator that construct each branch incrementally. The caller would then only take out the amount of branch needed and potentially cut out the last one in half before adding the trunk.

Going further, centering the tree inside the greeting card can be delegated to a string formatting function such as str.center or using the alignment specification of the string templating mini-language.


Lastly do not put code at the top-level of your module, it makes it harder to test or reuse it. Use an if __name__ == '__main__' guard instead.

Proposed improvements:

""" Seasons Greeting mail - ascii Christmas Tree """

import argparse
import shutil


MINIMAL_HEIGHT = 10


def command_line_parser():
    term_width, term_height = shutil.get_terminal_size()
    range_repr = f'[{MINIMAL_HEIGHT}-{term_height}]'

    parser = argparse.ArgumentParser(description=__doc__)
    parser.add_argument(
            '--height', '--tree-height', metavar=range_repr, type=int,
            choices=range(MINIMAL_HEIGHT, term_height+1), default=MINIMAL_HEIGHT,
            help=f'Height of the resulting tree including the trunk.')

    return parser


def branches():
    yield ')'
    branch_left = '(__'
    branch_right = '___)'
    while True:
        yield branch_left
        branch_left += '__'
        yield ' ' + branch_right  # Pad with space to get proper centering
        branch_right = '__' + branch_right


def tree(height):
    branches_iterator = branches()
    branch = next(branches_iterator)
    for _ in range(height - 3):  # Remove last (half) branch and trunk
        yield branch
        branch = next(branches_iterator)

    # Cut the branch in half
    width = len(branch)
    half_width = width // 2

    if branch.startswith('('):
        yield branch[:half_width] + ' ' * (width - half_width)
        yield ')'
        yield '(.'
    else:
        yield ' ' * (width - half_width) + branch[half_width + 1:]
        yield '('
        yield '.)'


def card(card_size):
    border = '_' * card_size
    yield f' {border} '
    yield enclose_in_borders('', card_size)
    for line in tree(card_size - 3):  # Remove header and footer lines
        yield enclose_in_borders(line, card_size)
    yield enclose_in_borders(border, card_size)


def enclose_in_borders(line, width):
    return f'|{line:^{width}}|'


if __name__ == '__main__':
    args = command_line_parser().parse_args()
    print('\n'.join(card(args.height)))
\$\endgroup\$
1
  • \$\begingroup\$ I didn't get the return f'|{line:^{width}}|', so i had to check the PEP 498 and PEP 3101 \$\endgroup\$
    – Circusgimp
    Commented Dec 23, 2019 at 7:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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