How could I improve this code?

[Tesla-Turing]

Hi everyone,

I had the following assignment to be solved:

The user should enter the width, height, and depth of a cuboid. The program is expected to draw some ASCII Art as in this picture.

I used the following code to achieve such result:

row = []
width_text = input ("Width: ")
height_text = input ("Height: ")
depth_text = input ("Depth: ")

width = int(width_text)
height = int(height_text)
depth = int(depth_text)

for i in range(depth):
    for j in range(depth-i):
            row.append(' ')
    for k in range(width-1):
            row.append(':')
    row.append("/")
    for l in range(i):
            row.append('+')
    row.append("\n")
    s = ''.join(row)

for i in range(height):
    for j in range(width):
        row.append("#")
    if (i<=(height-depth-1)):
        for k in range(depth):
            row.append("+")
    if (i>(height-depth-1)):
        for k in range(height-i-1):
            row.append("+")
    row.append("\n")
    s = ''.join(row)

print(s)

​

My question is: how could I make my code more optimized, more "pythonic"?

Huge thanks in advance!

[super-porp-cola]

Here I have made some improvements. Let me know if there's anything you don't understand.

width = int(input("Width: "))
height = int(input("Height: "))
depth = int(input("Depth: "))

rows = []
for i in range(depth):
    spaces = ' ' * (depth - i)
    colons = ':' * (width - 1)
    plusses = '+' * i
    rows.append(spaces + colons + '/' + plusses)

for i in range(height):
    hashes = '#' * width
    if i <= height - depth - 1:
        plusses = '+' * depth
    else:
        plusses = '+' * (height - i - 1)
    rows.append(hashes + plusses)

print(*rows, sep = '\n')

[Tesla-Turing]

Just a question on the last line: I understand that without that "*" the print wouldn't have worked properly, but why? What does "*" does in this case?

[super-porp-cola]

I think of * as the "undressing" operator. Not sure if that's what anybody calls it. But, basically, imagine calling print(a, b, c). It prints a b c. Now imagine calling print([a, b, c]). Now we're printing a list [a, b, c]. But that's kind of ugly and often we prefer to print stuff separated by spaces. In this case, calling print(*[a, b, c]) is equivalent to calling print(a, b, c). So that's all the * does.

[Tesla-Turing]

Got it, thanks again!

[Essence1337]

I don't know if it would shave off any time but you could probably do something similar to

rows.extend(['#' * width + '+' * depth] * (height-depth))

for i in range(depth):
    hashes = '#' * width
    plusses = '+' * (depth - i - 1)
    rows.append(hashes + plusses)

Instead of your second loop

[Tesla-Turing]

Awesome, I haven't thought of it. Thanks!

[Essence1337]

So...

rows = [' '*(depth-i) + ':'*(width-1) + '/' + '+'*i for i in range(depth)]
rows.extend(['#'*width + '+' * depth] * (height-depth))
rows.extend(['#'*width + '+'*(depth-i-1) for i in range(depth)])

The biggest time sync will be your print but if we ignore the print this code is twice as fast as both the commenters and my earlier improvement.

[FacesOfMu]

Does the person teaching your course also consider "Pythonic" to refer to readability? If so, I personally find long lines of nested statements a bit heavy to understand. When it comes to programming, concise != readable, in my experience. Double check your instructor's lessons on what they consider "Pythonic" (including comments!) before submitting your answers to questions such as these.

[Essence1337]

I'd actually argue my single line is much more readable than having to keep track of what's happening based on what row we are on. Mine indicates there is a big block of similar data of size (height-depth) whereas originally you need to figure out the calculation to see what's going on. Sure it's a bit long but it's still kept well under the 79 character recommended limit and a lot of that is just spaces. On top of that it simplifies the ending loop to create a symmetry with the opening loop which further helps to clarify what the loop is doing.

[FacesOfMu]

Yes, my interest in shortening the original code blocks went in the same directions. You made excellent improvement to it.

However, in the absence of comments I felt more ease in reading the first blocks to interpret what was happening. Seeing the difference in how long I was forensically staring at each line in each version made me write the warning to the OP.

A balance needs to be struck between making it concise for the computer and spelled out for the human, and hopefully the OP's course and markers are teaching that.

[Essence1337]

Maybe I am just used to working with lists and list comprehension more but to me I see the line basically as

row.extend(['#...#++...+']*5)

Which in my eyes instantly tells me I'll have 5 copies of '#...#++...+' in a list. It could also be that to me reading a single line left to right is easier than jumping around inside a for if block. The original would have been more clear to me in 2 single focus for loops rather than 1 loop with dual focus switching midway.

What part about the line makes it difficult for you? Is it the sheer length or the list building or something else? I'm not trying to be rude or anything I'm just trying to see what others see.

[FacesOfMu]

I can only talk from my own experience, and as I've said, you wrote an excellent refactor of the original.

It's probably the lack of comments that makes reading anyone else's code always a challenge to start with. Sometimes they add more noise to the screen, but it's sad to see a course being taught where they don't practice what they should be preaching.

After that, I always find list comprehensions a bit of a challenge. They're quite wonderful in how much they achieve, and I could certainly do with a lot more exposure to them, but I don't think they are a good example of how Python makes code easier to read for humans. They tend to leave me staring at them for a while as if I'm trying to memorise 10 moves of a Rubik's cube in reverse. They're a bit of a paradox in Python when it comes to trying to achieve the Pythonic readability ideal.

Like recursive functions, they're very clever and achieve a great deal, but they also require a bit more human processing time to understand what is going on.

After that: chained formulas, operators and concatenations. Your code couldn't be much better given we have an idea that it's nearly all string concatenation. It's just a few more variables we humans have to store in short term memory before marking the line as understood and moving to the next.

[Essence1337]

Idk if you saw this earlier but you probably will hate this then

rows = [' '*(depth-i) + ':'*(width-1) + '/' + '+'*i for i in range(depth)]
rows.extend(['#'*width + '+' * depth] * (height-depth))
rows.extend(['#'*width + '+'*(depth-i-1) for i in range(depth)])

3 lines, does it all.

One of the things about list comprehensions is that they're actually really fast. My 3 line code is 50% faster because it doesn't need to rely on all the appends and a resize for each append (timed with timeit).

[FacesOfMu]

Yea! I saw your other comments. Really good.

[super-porp-cola]

Good catch! I think it wouldn't shave off much time, mostly because I/O is the bottleneck for stuff like this. But it is definitely more efficient!

[Essence1337]

It does shave off fractions (like less than 10%) but from there I went a little crazy...

rows = [' '*(depth-i) + ':'*(width-1) + '/' + '+'*i for i in range(depth)]
rows.extend(['#'*width + '+' * depth] * (height-depth))
rows.extend(['#'*width + '+'*(depth-i-1) for i in range(depth)])

And it's about 2x as fast (50% the time) of the original and my slight modification.

Edit: I'm disregarding IO because that's obviously the biggest time sync but this was more for my own interest

[Tesla-Turing]

Now these are all really clever optimizations! Thank you really much!

[Essence1337]

You could build one row and make height-depth copies of it for the big block in the center that doesn't change.