Skip to content

Conversation

whitewaterdesign
Copy link
Contributor

@whitewaterdesign whitewaterdesign commented Mar 17, 2025

Refactor - Service method abstraction

A huge amount of code in the Service methods is a carbon copy. To avoid repetition am suggesting a functional abstraction, where we pass through a callback, and allow the abstraction to handle starting a new session, surfacing errors, logging etc. Appreciate that not everyone is a fan of a functional approach, so can always come up with a more OOP approach (which may reduce complexity at least in the usage). I believe functional makes more sense though in this service as there isn't any OOP. Also understand that this commit may be scrapped.

Positives - will save time in writing simple service methods.
Negatives - does add slightly more complexity

Why is this needed?

The main reasoning behind the abstraction is for every service method we need to:

  1. Test that the repo is being called correctly
  2. Test that when the repo method succeeds the metadata is being updated correctly
  3. Test that if the repo fails the error is being surfaced correctly

With the abstraction we know that 2 and 3 are working as expected, so for most tests we only need to check that the abstraction is being called correctly. We need a couple of "integration" tests to check the whole process, but it basically hugely simplify the whole test process, as for the majority of the tests we don't need to test 2/3 and we don't need the mongo mock in the test suite.

On writing the service methods it was very time consuming doing all the above, especially as we have pulled out the services into multiple files (due to SonarLint issues).

@whitewaterdesign whitewaterdesign requested a review from a team March 17, 2025 16:49
Copy link

Copy link
Contributor

@jbarnsley10 jbarnsley10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks good to me. Nice

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