Skip to main content
added 139 characters in body
Source Link
sunny
  • 1.8k
  • 1
  • 13
  • 29

I agree with the two answers posedposted above. There are a few additional points I would add:

This renaming also shouldn't impact your for loop variables much. min -> border_width and max -> count - 1 - border_width

*Note I typed out the code above without running it so please let me know if there is a syntax error I can fix if you can't figure it out.

I agree with the two answers posed above. There are a few additional points I would add:

This renaming also shouldn't impact your for loop variables much. min -> border_width and max -> count - 1 - border_width

I agree with the two answers posted above. There are a few additional points I would add:

This renaming also shouldn't impact your for loop variables much. min -> border_width and max -> count - 1 - border_width

*Note I typed out the code above without running it so please let me know if there is a syntax error I can fix if you can't figure it out.

Source Link
sunny
  • 1.8k
  • 1
  • 13
  • 29

I agree with the two answers posed above. There are a few additional points I would add:

1. Your massive control statement can be simplified:

if (min <= max and min <= i and i <= max and min <= j and j <= max) and (i == min or i == max or j == min or j == max)

In particular you can delete min <= max

You can also rewrite the final condition, (i == min or i == max or j == min or j == max), more simply as (i in (min, max) or j in (min, max))

Finally you can rewrite min <= i and i <= max as min <= i <= max which is both shorter and easier to understand.

2. Why not use a list comprehension to replace your inner for loop?

Seems like you can rewrite

        for j in range(0,count): #this loop goes from column 0 to column N, where N is the value entered by the user. This will be responsible for printing the stars in the square, from column to column.
            if (min <= max and min <= i and i <= max and min <= j and j <= max) and (i == min or i == max or j == min or j == max): #this branch checks if ceertain cinditions are met. then prints * if they are. The conditions are explained below.
                print("*",end="") #prints * without a line break after every iteration of its loop
            else:
                print(".",end="") #prints . without a line break after every iteration of its loop

as something like (I haven't tested this):

def should_print_asterisk(i, j, min, max): return (min <= i <= max and min <= j <= max) and (i in (min, max) or j in (min, max)))

and then inside the i based for-loop:

print(["*" if should_print_asterisk(i,j,min,max) for i in range(0, count) else "."])

3. Check the logic of your control statements to keep them efficient

You have two if statements that are mutually exclusive? Why not refactor this into an if...elif combo?

4. Refactoring logic to eliminate variables

The information contained in your pattern function variables min and max is actually identical. They both represent an offset from the outer edge of the shape. Consider whether you should simply have one variable, something like border_width. In this case, your control statement if run < count/2 ... could be reduced to:

            if run < count / 2: #in this branch, if the iterations of the boxes < half of the user input, the variable min will be incremented by 1
            border_width += 1
        else:
            border_width -= 1

This renaming also shouldn't impact your for loop variables much. min -> border_width and max -> count - 1 - border_width