BadgerHobbs / BeatSage-Downloader

Unofficial download tool for Beat Sage custom Beat Saber level generator.
44 stars 6 forks source link

Refactoring code structure #4

Open HerringTheCoder opened 4 years ago

HerringTheCoder commented 4 years ago

Why: I think this app is great and will be very useful for growing BeatSage community, but it could get a bit more readable, so it can be maintained and expanded by author and open source community in the future. The changes proposed here might feel slightly huge at first, but I think it might be a good step in the right direction.

What this PR does:

  1. Divides code structure into:

    • Views (containing xaml windows and xaml.cs logic files)
    • Models (classes)
    • Services (mostly in form of static helpers as of now) This code structure relates more to typical MVVM architecture used in WPF apps.
  2. Extracts Download and DownloadManager classes (Models) into separate files

  3. Adds following services:

    • YoutubeService (responsibility of Importing and Retrieving playlists, could still be expanded)
    • CustomLevelService (responsibility of requesting level creation by CreateFromFile, CreateWithLocalMP3Download and Create methods)

The steps above mean that xaml.cs files are now much less responsible for managing business logic and contain mostly methods related to click events.

  1. Removes unnecessary usings and sorts them alphabetically, removes some redundant values.

  2. Updates .Net Framework to v.4.7.2 and implements latest PropertyChanged.Fody, which helped simplifying binding syntax.

What this does not do:

  1. It does not interfere with methods logic of original author.
  2. It does not divide existing methods into smaller ones.
  3. It does not embrace real MVVM (lack of ViewModels, binding to Models)
  4. It does not repair bug related to removing list element when it's processed.

What I think could be further improved:

  1. Binding settings checkboxes state into boolean fields (that's where ViewModel could be useful) to avoid multiple checks against the settings in AddDownloadWindow.xaml.cs
  2. Previous settings could be then parsed in separate service as a List and checked in more robust way.
  3. Cleaning methods (shorter syntax, removing unnecessary values further, dividing responsibilites further)
  4. Using Dependency Injection for Services

Tested functionality with playlists, multiple links, various settings and paths.

Feel free to squash merge if you like the changes or discuss them further.