topcoderinc / va-kidney-ios

Mobile Kidney App for VA
2 stars 2 forks source link

Code Quality Issue - CachingServiceApi has some non-protocol methods #41

Closed talesforce closed 6 years ago

talesforce commented 6 years ago

As noted from previous challenge review

CachingServiceApi has some non-protocol methods (e.g. getProfile) and is used as singleton in some cases across the app. It should be always referenced through protocol injection-like variable for consistency. You should extract database interactions from CachingServiceApi. Instead you have an one large class that does everything, with scale it will become increasingly harder to support

N1k1tung commented 6 years ago

For clarification: ultimately CachingServiceApi implies a backend interaction,

1) all occasions where CachingServiceApi is used for backend interaction should be replaced with protocol instance usage. 2) all logic from CachingServiceApi class related purely to database handling should be extracted to a different class. 3) all cases where CachingServiceApi is used only to interact with database need to be replaced with usage of that different class