Refactor: Separate File Identification, Database Operations and Download in different packages

[sieberss]

Description

Problem:

I wanted to implement parallel downloading and inspected the code for this issue. But I could not find a single method that executes sequential download and could be changed for parallel work.
There are several files where some part of the download action happens, but this is always mixed with Database manipulation and and handling of URLs. So I can't identify the code which can be parallelized.

Furthermore there are some very complex methods that need refactoring to be understandable.
I executed a code check with Sonarcloud that considered several methods too complex. And many other issues were found. Here you can see the result:
[https://sonarcloud.io/project/issues?impactSeverities=HIGH&issueStatuses=OPEN%2CCONFIRMED&id=sieberss_Drifty]

My Suggestion

• extract all database operations and URL handling from existing code to new Classes and provide them as public methods
• put these classes in separate packages in Core module
• use package and class names only once, combine files with same name at one central place
• rename some classes and packages to better express their purpose

Advantages

• code becomes easier to understand
• it will be faster to find the code where changes have to take place
• better maintenance
• changes in database (e.g. other DBMS, use of frameworks like Hibernate) can be made with low programming effort
• possibility to integrate Drifty in something like a Spring application

Additional information

No response

Do you want to work on this issue?

Yes

[github-actions]

Hello 👋! Thank you very much for raising an issue 🙌! The maintainers will get back to you soon for discussion over the issue! 🚀

Meanwhile you can also discuss about the project in our Discord Server 😀

[SaptarshiSarkar12]

@sieberss Thank you for raising this issue. I appreciate the suggestions you have listed. It will indeed decrease the maintainability burden. I am assigning this issue to you.

[sieberss]

Thank you for assigning :)

First off some naming issues. Can you explain me the purpose of these packages and classes? Suggestions for a more verbose name are very welcome.

Module Core:

1. init (package)
2. Environment
3. preferences (package)
4. AppSettings
5. Get
6. Set
7. Clear
8. properties (package)
9. Program
10. support (package)
11. Job
12. JobHistory
13. Jobs
14. utils (package)
15. Utility

Module CLI

1. init (package)
2. Environment (extends init.Environment ??)
3. support (package)
4. Constants (extends support.Constants ??)
5. utils (package)
6. MessageBroker (extends utils.MessageBroker?? Difference to MessageBroker in Core?)
7. Utility (extends utils.Utility?? Difference to Utility in Core?)

Module GUI

1. gui (package containing the same packages as module CLI) - redundant?
2. ui (package) - okay without package gui
3. Splash

[sieberss]

Packages are most commonly named in reverse domain style. We should use such a prefix for all drifty packages. What would you like?
Then the package "main" with only one class can be removed from all modules.

[SaptarshiSarkar12]

First off some naming issues. Can you explain me the purpose of these packages and classes? Suggestions for a more verbose name are very welcome.

Yeah, sure.

Module Core:

1. init (package)

This package deals with the initialisation tasks like setting up the environment required by both CLI and GUI.

2. Environment

This class checks the presence of yt-dlp and ffmpeg standalone binaries and copies them to Drifty application directory (.drifty in linux and unix systems) and creating necessary database tables. It also contains a method for graceful termination with revoking Spotify access token and updating yt-dlp.

3. preferences (package)

This package contains classes and interfaces related to some system settings like last time yt-dlp update was checked, etc.

4. AppSettings

This is a centralized class containing shared static instance of GET, SET and CLEAR classes for systematic access without invoking getInstance() methods multiple times.

5. Get

This class contains some methods to easily fetch the required preferences/settings.

6. Set

Same as above but for setting the preferences.

7. Clear

Same as above but for clearing settings. Useful in case of easily revoking stored Spotify access token.

8. properties (package)

This package holds classes which change the behaviour of the application. Drifty heavily depends on OS and Program class. For example, depending on OS, the executable name of yt-dlp and ffmpeg differs.

9. Program

This class contains methods to fetch and set some program variables like yt-dlp executable path.

10. support (package)

This package contains classes that support the download process like metrics and configuration of the download process, etc.

11. Job

This class contains necessary details of each download job like detected filename, download link, etc.

12. JobHistory

This class contains methods for adding a job to download history, checking the existence of a particular download job in history, etc.

13. Jobs

This class just stores a list of download tasks to be done.

14. utils (package)

This package simply contains utility classes.

15. Utility

This class contains methods that help the other packages and modules for basic functions like checking whether a link is of YouTube, Spotify or a general link, etc.

Module CLI

1. init (package)
2. Environment (extends init.Environment ??)

This package contains Environment class which overrides two methods of init.Environment class for setting and fetching CLIMessageBroker useful for sending all different kinds of messages (INFO, WARNING, ERROR) to a single channel (System.out by default but can be set to a file also).

3. support (package)
4. Constants (extends support.Constants ??)

This class contains some CLI specific constants in addition to the common constants used in CLI and GUI.

5. utils (package)

This package contains some utility classes specific to CLI.

6. MessageBroker (extends utils.MessageBroker?? Difference to MessageBroker in Core?)

This class contains some methods to simplify code and improve readability by avoiding invocation of redundant values passes to sendMessage function based on context.

7. Utility (extends utils.Utility?? Difference to Utility in Core?)

This contains methods specific to CLI like cli input validation whose code might be repeated otherwise.

Module GUI

1. gui (package containing the same packages as module CLI) - redundant?

The gui package contains packages and classes required for GUI specific tasks.

2. ui (package) - okay without package gui

The ui package has been kept out of gui.* packages because it doesn't inherit any part of the core module which contains common parts of both CLI and GUI. So, ui package is completely dedicated to GUI window and JavaFX scene handling.

3. Splash

This class handles the splash screen (a JavaFX preload).

@sieberss I hope you have got a clear idea of what packages are responsible for what tasks and the intuition for creating separate packages in other modules extending functionality of the common packages. I am out of any ideas for names of these classes. Let me know your thoughts on this.

[SaptarshiSarkar12]

Packages are most commonly named in reverse domain style. We should use such a prefix for all drifty packages. What would you like?

Yeah, definitely a good idea. Just kept that idea aside initially, for simplicity.

Then the package "main" with only one class can be removed from all modules.

I think that will break the modular structure required for build process. We are using a module-info.javafile which declares necessary exports and dependencies required. We strictly cannot remove that file. You may try removing main package and run the build process once to verify it doesn't fail.

[sieberss]

Module Core:

1. init (package)

This package deals with the initialisation tasks like setting up the environment required by both CLI and GUI.

What about setup? Would give me a better idea.

2. Environment

This class checks the presence of yt-dlp and ffmpeg standalone binaries and copies them to Drifty application directory (.drifty in linux and unix systems) and creating necessary database tables. It also contains a method for graceful termination with revoking Spotify access token and updating yt-dlp.

Maybe tools, binaries, external or something similar?

BTW, what tasks are executed by these binaries? AFAIK yt-dlp can download itself. What is the intended additional benefit of Drifty?

3. preferences (package)

This package contains classes and interfaces related to some system settings like last time yt-dlp update was checked, etc.

settings?

4. AppSettings

This is a centralized class containing shared static instance of GET, SET and CLEAR classes for systematic access without invoking getInstance() methods multiple times.

What are these instances needed for? Wouldn't static methods in a class be enough?