refactoring stuff #201
refactoring stuff #201
Conversation
|
test |
![ed-codegen[bot]](https://avatars.githubusercontent.com/u/28070492?s=60&v=4){kind=small}
| video_files = [f"screen-{i}.mp4" for i in range(1, 5)] | ||
| vlc_processes = start_videos(video_files) | ||
|
|
||
| # Schedule the corruption process |
There was a problem hiding this comment.
Really good job at refactoring the final.py file. The way you have condensed multiple functions into more streamlined versions and added docstrings for clarity is a major improvement. Also, wrapping the application in a main() function and checking for __main__ gives it a more professional structure. 🚀
| to Johnny in the narrative when he loses his own memories due to the overload of | ||
| information in his brain. | ||
|
|
||
| the thing with falling in love for someone that is 1000km away is that love, |
There was a problem hiding this comment.
It seems a non-related personal opinion has been added to README.md in line 23-24.
While it's important for developers to express themselves, this comment doesn't appear to relate to the project. Omitting it may make the document clearer and more professional to readers. 🧐
| to Johnny in the narrative when he loses his own memories due to the overload of | ||
| information in his brain. | ||
|
|
||
| the thing with falling in love for someone that is 1000km away is that love, |
There was a problem hiding this comment.
It would be helpful to include a bit more context or information about these statements. Are they intended to communicate particular thoughts to users? Are these part of some kind of narrative within your project?
|
|
||
| return video_files | ||
| def process_videos(videos): | ||
| """Corrupts a list of videos.""" |
There was a problem hiding this comment.
Good move removing the hardcoding of the filenames here. By accepting the files as a parameter, you have made your function more versatile. This would support a wider variety of use cases.
|
|
||
| return vlc_processes | ||
| """Plays a list of videos.""" | ||
| return [play_video(i, video_file) for i, video_file in enumerate(video_files)] |
There was a problem hiding this comment.
Great refactor to the start_videos function. Breaking the code into a list comprehension makes it cleaner and easier to understand.
| video_files = process_videos() | ||
| vlc_processes = start_videos(video_files) | ||
| # Schedule the corruption process | ||
| schedule.every().day.at("11:30").do(lambda: scheduled_task(video_files, vlc_processes)) |
There was a problem hiding this comment.
Given this is an external scheduling dependency and is global, have you considered how it would perform under testing conditions? It could be hard to test this main function as it uses a global instance of 'schedule' and real time. Consider encapsulating the schedule within a class or a method to make it more testable.
|
|
||
| # testing 1 2 3 | ||
|
|
||
| # test |
There was a problem hiding this comment.
In this test file, there's no test cases defined. Remember unit tests are essential in ensuring added functionalities work as expected. Consider adding some tests.
No description provided.