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

Refactoring Suggestions #29

Open
scott-coates opened this issue Feb 7, 2023 · 0 comments
Open

Refactoring Suggestions #29

scott-coates opened this issue Feb 7, 2023 · 0 comments
Assignees

Comments

@scott-coates
Copy link
Contributor

scott-coates commented Feb 7, 2023

@waldothedeveloper You should be very proud of completing the v1. Even if you had written poor quality code (you didnt', it's really good for a v1), it's better to ship a v1 with poor code than to ship nothing at all. Great job.

I'm adding some of my feedback here, per your request from #1 (comment).

Organization

image
I really like how you organized the app, it's intuitive and sensical. However, when I was looking for the code that produces the playlist, aka the core business logic, I couldn't easily determine where it exists by looking at this structure. Conventionally, it is common to create a folder called lib or app, and this is where your core app logic would reside. This makes it easier for new developers to your project to quickly identify which parts of your app reside within the folder structure.

Testability

A lot of the core business logic exists in React Hooks. This is a great way to start, but it will make your app harder to test. This is because, for the most part, you could put a lot of your playlist filling logic in a regular js function, absent of any react (hooks, state, etc). It is easier to write tests when the System Under Test (SUT) is a simple js function, not a React Component. For your v2, I would split the logic into its own file, and then import that logic into your react components.

SOLID

image
The fetcher files could probably be DRYd up (Don't repeat yourself from the SOLID principles)

NOTE: I do not prescribe following SOLID principles when shipping a v1. Because you've completed your v1, you should consider refactoring with SOLID principles.

useSWRImmutable

Can you please walk me through this and how it works? I'm not too familiar with it.

@waldothedeveloper waldothedeveloper self-assigned this Feb 8, 2023
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

No branches or pull requests

2 participants