3
\$\begingroup\$

I need to write a program where the user can switch between rect positions. rects position re-ordering relevant for activating other functions that not relevant for this question.

My algorithm works, but I'm sure there are better ways in terms of logical structure, memory and runtime. Unfortunately I did not find it myself. I need to compare my solution with other solutions (for self-esteem and to continue with my project).

class small_rect():
 def __init__(self,y,color,text): 
 self.color = color
 self.rect = pygame.Rect(200,100,250,50)
 self.rect.center = (250,y)
 self.text = font.render(text,True,(255,255,255))
 
 def draw(self,rect_func):
 text_rect = self.text.get_rect(center=(rect_func.center))
 pygame.draw.rect(screen,self.color,self.rect)
 screen.blit(self.text, text_rect)
class big_rect():
 def __init__(self,y): 
 self.color = (25,25,25)
 self.rect = pygame.Rect(180,80,260,70)
 self.rect.center = (250,y)
 def draw(self): 
 pygame.draw.rect(screen,self.color,self.rect,4)
 
r1 = small_rect(100,(100,100,100),'TEXT1')
r2 = small_rect(200,(200,20,20),'TEXT2')
r3 = small_rect(300,(20,200,20),'TEXT3')
r4 = small_rect(400,(20,200,200),'TEXT4')
R1 = big_rect(100)
R2 = big_rect(200)
R3 = big_rect(300)
R4 = big_rect(400)
small_rect_lst = [r1,r2,r3,r4]
big_rect_lst = [R1,R2,R3,R4]
 
run = True
while run:
 clock.tick(fps)
 screen.fill((15,150,250))
 pos = pygame.mouse.get_pos()
 for i,rec in enumerate(small_rect_lst):
 if rec.rect.collidepoint(pos):
 pygame.draw.rect(screen,(250,250,250),rec.rect,4)
 
 if pygame.mouse.get_pressed()[0] :
 rec.rect.centery = pos[1]
 
 if i > 0 and rec.rect.colliderect(big_rect_lst[i-1].rect):
 small_rect_lst[i-1].rect.centery = big_rect_lst[i].rect.centery
 rec.rect.centery = big_rect_lst[i-1].rect.centery
 small_rect_lst[i-1],small_rect_lst[i] = small_rect_lst[i],small_rect_lst[i-1]
 if i < 3 and rec.rect.colliderect(big_rect_lst[i+1].rect):
 small_rect_lst[i+1].rect.centery = big_rect_lst[i].rect.centery
 rec.rect.centery = big_rect_lst[i+1].rect.centery
 small_rect_lst[i+1],small_rect_lst[i] = small_rect_lst[i],small_rect_lst[i+1]
 
 else:
 rec.rect.centery = big_rect_lst[i].rect.centery
 rec.draw(small_rect_lst[i].rect)
 
## for j in big_rect_lst:
## j.draw()
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 run = False
 pygame.display.update()
pygame.quit()
toolic
16.9k6 gold badges30 silver badges224 bronze badges
asked Dec 28, 2021 at 21:21
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

DRY

The following code has a repeating pattern:

R1 = big_rect(100)
R2 = big_rect(200)
R3 = big_rect(300)
R4 = big_rect(400)

The repetition can be eliminated using a loop, such as:

big_rect_lst = []
for i in range(1, 5):
 big_rect_lst.append(big_rect(i*100))

This has the side benefit of eliminating the temporary, generically-name variables: R1, R2, etc.
It also allows the code to be more scalable.

You can also eliminate the temporary variables (r1, r2, etc.) by directly adding to the array:

small_rect_lst = [
 small_rect(100,(100,100,100),'TEXT1'),
 ...
]

Documentation

Add a docstring at the top of the file to summarize the purpose of the code. Also add docstrings to describe the function inputs.

Naming

small_rect_lst would be better named as small_rects. The _lst suffix is vague; you probably mean a list, in which case you may as well have written the word "list". The _lst suffix is unnecessary if you simply pluralize rects to denote a list of them.

The same is true for big_rect_lst.

Comments

Remove this commented-out code because it just adds clutter:

## for j in big_rect_lst:
## j.draw()

Space

The code is hard to read without space after the commas. The black program can be used to add consistent spaces.

answered Oct 24, 2024 at 11:41
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.