janmechtel / rockettype

A windows keylooger to track typing statistics like WPM
MIT License
3 stars 0 forks source link

Refactoring - using env[] everywhere is weird #36

Open janmechtel opened 4 years ago

janmechtel commented 4 years ago

Please suggest ways to restructure the code to avoid env[""] use. Or confirm that this is best practice.

I'd like a list of suggestion how you'd re-factor the project at this stage + rough estimate of effort.

We'll then talk through the list and decide together which refactoring to go for.

BearVictor commented 3 years ago

Code Refactoring Recommendations

  1. Folder DEBUG has to be in folder Logs. It is a bad resolution to create a new folder only for change mode in the program (Logs created after the first start program). TODO: make DEBUG <true/false> as a program input parameter and parse this in the program. It is cleaner and flexible.

  2. Not valid paths to statistics file in the code. All data is written into Logs/key_log_wpm.txt, not in Logs/key_log.txt. So, logger_stats.py also has a bad path. TODO: All file paths should be in one place, and defined like a global variable. In this case, we can easily and quickly change this.

  3. Change .txt extension to .csv? Add header line creation in the code (currently seems to be manually created).

  4. A lot of keys are in global env={} dict while used only in function get_active_process. This function takes the name of the active application.

TODO: create a class and relocate all variables from env={} and logic from get_active_process. Also, remove this class in another file. This helps structure the program and make it more flexible. For example: when wanna make this program for another OS, just need to create another class that takes the application name. And in the main file change only 1 line.

Example:

class <WINDOW>:
include:
    get_active_process()
    env['OpenProcess']
    env['PROCESS_QUERY_INFORMATION']
    env['pid']
    env['MAX_PATH']
    env['GetProcessImageFileName']
    env['CloseHandle']
  1. Make the main class like a mediator, where the use object from another classes like GUIand <WINDOW>. This also avoids global variable env={}.

TODO: Create separate class for all logic related to key press:

Example

class <PressedKey>:
Include:
on_press()
async_on_press()
env['words']
env['previous_time']
env['debug_mode']

So, creating a few new classes helps avoid env={} variables. That helps to make the project more readable and flexible, clean, and easy for the new implementation.

General recommendations

We recommend using the linter library pylint. Pylint is a tool that checks for errors in Python code, tries to enforce a coding standard, and looks for code smells. The highest rate is 10.

Screenshot 2021-02-19 at 15 44 41

This project has a 1.5/10 rate. It is too bad, and not up to standards. A lot of variables have a bad name and naming style, a lot of them are not used too.

Also, we recommend decompressing big functions into small ones.

Found bugs Sometimes the lock file is not deleted properly so you can’t start the program.