Avoid pyramid branching

nai commented

2025-05-15 18:53:52 -07:00

Collaborator

while True:

It's usually easier to read if the pyramid is inverted, for example you did well here:
https://git.waifuism.life/waifu/kemoverse/src/branch/master/bot/add_character.py#L25

especially in python, where you don't have brackets.

As a tule of thumb, just avoid doing more than 2 nestings in a function, and if it happens: invert the control flow

https://git.waifuism.life/waifu/kemoverse/src/commit/b8493440bdf8dbb87535f403a49135ae05f89c54/bot/bot_app.py#L29 It's usually easier to read if the pyramid is inverted, for example you did well here: https://git.waifuism.life/waifu/kemoverse/src/branch/master/bot/add_character.py#L25 especially in python, where you don't have brackets. As a tule of thumb, just avoid doing more than 2 nestings in a function, and if it happens: invert the control flow

nai added the

Refactoring

label 2025-05-15 18:53:52 -07:00

waifu was assigned by nai

2025-05-15 18:54:01 -07:00

waifu commented

2025-05-15 19:57:33 -07:00

Owner

Okok I see the problem, I remembering Linus saying more than 3 nests in a function meant you needed another one. So I could write another function that does the notification stream, and keep the configuration loading in main so it can pass it over to the other function.

nai commented

2025-05-16 05:39:34 -07:00

Author

Collaborator

You don't even need more functions, just consider the starting point:

The whole while true is entirely wrapped in one huge if notifications:. So you can safely invert it, and just do:

if not notifications:
  continue
  
notifications.reverse()
new_last_seen_id = last_seen_id
for notification in notifications:
  // 2 spaces!

instead of

if notifications:
  notifications.reverse()
  new_last_seen_id = last_seen_id
  for notification in notifications:
     // now 4 spaces

Everything that used to be within the if is now on the same level with it, so you just avoided a nesting with literally no effort.

B-but, my sleep(5) !

It can move to right before notifications = client.i_notifications()

You don't even need more functions, just consider the starting point: - The whole `while true` is entirely wrapped in one huge `if notifications:`. So you can safely invert it, and just do: ``` if not notifications: continue notifications.reverse() new_last_seen_id = last_seen_id for notification in notifications: // 2 spaces! ``` instead of ``` if notifications: notifications.reverse() new_last_seen_id = last_seen_id for notification in notifications: // now 4 spaces ``` Everything that used to be within the `if` is now on the same level with it, so you just avoided a nesting with literally no effort. > B-but, my sleep(5) ! It can move to right before `notifications = client.i_notifications()`

nai commented

2025-05-16 05:48:30 -07:00

Author

Collaborator

and yes, as you said, you can alternatively move everything to a function, so it's something like

notifications = client.i_notifications()
process_notifications(notifications)
time.sleep(5)

that works too and is a way to approach it, whatever you like better

and yes, as you said, you can alternatively move everything to a function, so it's something like ``` notifications = client.i_notifications() process_notifications(notifications) time.sleep(5) ``` that works too and is a way to approach it, whatever you like better

VD15 added reference dev

2025-05-21 11:07:55 -07:00

VD15 commented

2025-05-21 11:31:28 -07:00

Collaborator

Think this can be closed when dev is next merged into master as most of the pyramid branching in the repository has been addressed over the past few days.

Think this can be closed when [dev](https://git.waifuism.life/waifu/kemoverse/src/branch/dev) is next merged into [master](https://git.waifuism.life/waifu/kemoverse/src/branch/master) as most of the pyramid branching in the repository has been addressed over the past few days.

VD15 closed this issue

2025-05-24 14:46:45 -07:00

Rows
Columns

Avoid pyramid branching #4