Remove unused Serializable instances

[Profpatsch]

These two classes are not serialized anywhere (anymore).

What is it?

• [ ] Bugfix (user facing)
• [ ] Feature (user facing)
• [x] Codebase improvement (dev facing)
• [ ] Meta improvement to the project (dev facing)

Due diligence

• [ ] I read the contribution guidelines.

[sonarqubecloud[bot]]

Quality Gate failed

Failed conditions

C Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[HatakeKakashri]

Hi @Profpatsch,

Thank you for your contribution and the thought put into this pull request! I appreciate your effort in addressing the identified concern.

Considering the upcoming migration to Kotlin and Compose, it will likely address the concerns you've raised, making this change unnecessary in the long run.

To focus our collective efforts optimally, I recommend: - Closing this pull request: Let's close this PR for now. Once we have a clearer picture of how the migration handles the relevant functionality, we can revisit this PR or, if needed, create a new one tailored to the new framework. - Focusing on reported bugs: In the meantime, your expertise would be highly valuable in addressing existing bugs in the codebase. This would contribute significantly to improving the project's stability and user experience.

Additional feedback: - Context and justification: While the proposed change seems well-intentioned, providing clear context and rationale would make the review process smoother. This could include clearly articulating the reasoning behind changes, especially those potentially impactful. Detail steps taken to validate their necessity (e.g., verifying serialization usage). - Potential impact: It's always helpful to explicitly mention the potential impact of changes, especially those with broader implications. This allows reviewers to assess the trade-offs more effectively.

I hope this perspective helps! Feel free to discuss any questions or concerns you might have.

CC: @opusforlife2, @AudricV

[opusforlife2]

Hey snaik, good to see the reviews starting! Also, we get notifications, so no need to ping by username.

---

I'm no dev, but for tiny code quality improvements like this, is it worth thinking about them too much before merging? As long as it's clear that there will be no regression, I mean.

[HatakeKakashri]

Hey snaik, good to see the reviews starting! Also, we get notifications, so no need to ping by username.

SG. Will avoid tagging by username.

I'm no dev, but for tiny code quality improvements like this, is it worth thinking about them too much before merging? As long as it's clear that there will be no regression, I mean.

While the code change seems small, it's essential to carefully consider its potential impact before merging, even in the absence of obvious regressions. Here's why:

• Wide Usage of PlayQueueItem: The PlayQueueItem class plays a significant role in the app's functionality. Any modification to it, however minor, has the potential to ripple through numerous parts of the code, potentially leading to unexpected issues.
• Serialization Nuances: Changes to serialization can be particularly delicate. They can affect how data is stored and transmitted, potentially causing compatibility issues with existing data or integrations. Verifying the usages of the usages is crucial to ensure seamless functionality.

[Profpatsch]

Ehm, this is a pretty easy simplification that does not touch serialization. It just removes unused implementations. It compiles, it’s good to go.

[Stypox]

Unfortunately it's not true that if it compiles, then it works. This is what I get when I start the player. I agree with @snaik20 , it's better to avoid risking to introduce regressions if the improvements to the code are this small. But thank you anyway!

Serialization failed for:
java.io.NotSerializableException: org.schabi.newpipe.player.playqueue.PlayQueueItem
at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1240)

[Profpatsch]

Is it seriously resolving instances at runtime? what a horrible design