Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Simplify if-else logic, cleanup and other refactors #75

Closed
wants to merge 9 commits into from
Closed

Conversation

Uzaaft
Copy link

@Uzaaft Uzaaft commented Nov 15, 2022

WIP PR.

The purpose of the PR is to cleanup things, get some easy speed wins, and make if-else logic easier to understand.
Just want some of your input before I mark this as not a draft.

  1. I see some instances where variables are immediately returned after variable creation. Would it be alright to refactor these to just return immediately?
    ex:
def foo(a):
	b = a ** 2
	return b

can be turned into:

def foo(a):
	return a ** 2
  1. Swap if-else statements with if-else expressions:
    For.example:
def _set_view(self):
        if self.box:
            self.width = self.size.width - 4
        else:
            self.width = self.size.width

to:

def _set_view(self):
        self.width = self.size.width - 4 if self.box else self.size.width

@kraanzu
Copy link
Owner

kraanzu commented Nov 18, 2022

Hey @Uzaaft ,

Really sorry for the late reply!
Ig I didn't see your message properly before!

Thanks for all the changes you're making along the way!
Just a heads up that I'll be merging the redesign branch soon and will merge these if there's not a conflict!

So It'll be great if you can work on that :)

I see some instances where variables are immediately returned after variable creation. Would it be alright to refactor these to just return immediately?

Yes of course, as long as it does make the code look overcomplicated it's fine :)

@kraanzu kraanzu closed this Apr 28, 2023
@Uzaaft
Copy link
Author

Uzaaft commented Apr 28, 2023

No worries, I won't be submitting a new PR, I moved on to my own tool instead due to the delay. Good luck on the future development. 😄

@kraanzu
Copy link
Owner

kraanzu commented Apr 29, 2023

Hey, @Uzaaft sorry for not describing why I closed the PR.

Thank you so much for doing the refactoring. Really appreciate it!
But the thing is, I made a lot of changes in the code when I pushed the new release #87

image

So the code which you refactored is not really in there anymore :(
I'm so sorry for that!

Good luck with your new tool 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants