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

Added comments to main.py and removed hardcoded values from display.py to globals.py #183

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hlai1
Copy link

@hlai1 hlai1 commented Dec 8, 2023

Changes

  • added comments to main.py to increase readability
  • removed the hardcoded values from display.py (numBars, delay, time_taken, etc.) and moved them to globals.py
  • imported hardcoded values from globals.py to display.py for usage

Why (resolves #176)

  • Increases readability of code
  • Makes code cleaner and more module
  • Global definitions are easier to reuse across multiple modules

How

  • Went through main.py to understand logic, then commented the file
  • Created globals.py file, moved the hardcoded values from display.py over
  • Imported the values from globals.py into display.py for usage throughout the file

Testing

  • Ran the data visualizer again, confirmed that functionalities still operated the same as before (timer, switching sorting types, etc.)

Comment on lines +8 to +9
start_time = 0
time_taken = 0
Copy link
Contributor

@idomeisner idomeisner Dec 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think start_time and time_taken need to be globals.
They are relevant only for DisplayBox, why not add them as class or instance variables?
There are several options for how to implement this.
For example, you can set start_time when starting a new sort and update time_taken when calling the update method.
You can also save only start_time and directly do something like:
self.text = str(time() - start_time)[:6] + ' seconds'

@@ -62,6 +65,21 @@ def update(self, event):
if event.key == pygame.K_BACKSPACE: self.text = self.text[:-1]
elif event.unicode.isdigit() : self.text += event.unicode

class DisplayBox(InputBox):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure sure DisplayBox is a good name, all boxes display something.
Looking at the other names TextBox, SlideBox, DropdownBox, ...
Maybe TimeBox or something like that would be better.

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.

Improve main.py and remove hardcoded values from display.py
2 participants