Skip to content

Conversation

@ShaneMander
Copy link
Collaborator

@ShaneMander ShaneMander commented Mar 25, 2025

Still a draft.

Breaking apart some of the service classes and introducing smaller more specific ones:

  • IEventUserService
  • IUserFriendTagService
  • IFriendUserService

@ShaneMander ShaneMander requested a review from Daggerpov March 25, 2025 05:07
@Daggerpov
Copy link
Owner

Daggerpov commented Mar 25, 2025

I think what you have is great! It's clear why we need to separate methods that are related to different parts (basic CRUD, participation, etc.) into different classes.

I think splitting it up entirely makes the code a little harder to trace, though, and would personally prefer to have a hierarchy between the classes, where EventService is essentially the highest-up class that the controllers communicate with (as we had before), that then calls down to EventUserService through a Facade pattern, instead of completely separating the two.

So, what you have is 90% what I'd like, and I'm of course willing to talk through this solution, but if we were to go with my idea, that'd simply involve adding the methods back to EventService, and having them call down to EventUserService, like so:

private List<UUID> getParticipatingUserIdsByEventId(UUID eventId){
    return eventUserService.getParticipatingUserIdsByEventId(eventId);
}

Instead of the controllers directly calling different sub-service methods, because that can be trickier to track.

I think going with this approach, we can keep the simplicity we had before of the controllers and external classes calling the same methods, but internally, we're implementing those methods' logic in a 'deeper' service class layer through the Facade pattern, to keep the external usage the same and avoid all of the external refactoring that's coming with these changes.

With this approach, the only class you'd have to modify would be EventService itself, since externally, it'd appear the same.

…into service-refactor

# Conflicts:
#	src/main/java/com/danielagapov/spawn/Services/Event/EventService.java
#	src/main/java/com/danielagapov/spawn/Services/User/IUserService.java
#	src/main/java/com/danielagapov/spawn/Services/User/UserService.java
#	src/test/java/com/danielagapov/spawn/ServiceTests/EventServiceTests.java
#	src/test/java/com/danielagapov/spawn/ServiceTests/UserServiceTests.java
@ShaneMander ShaneMander changed the base branch from staging to main April 5, 2025 06:23
@Daggerpov Daggerpov closed this Jul 5, 2025
@Daggerpov Daggerpov deleted the service-refactor branch July 5, 2025 04:28
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

Successfully merging this pull request may close these issues.

3 participants