Timeline for Is logging next to an implementation a SRP violation?
Current License: CC BY-SA 3.0
18 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Sep 28, 2021 at 23:29 | comment | added | ZombieTfk | The fact it requires twice as many classes is a failure of the language, not the code given here, which is very pure and correct way to do this in OOP. If someone seriously wants logging for every class (And normally you'd just want it around logic that causes side effects, which shouldn't be many if you have clear and well defined boundaries) then AOP would be a better approach to solve this issue without breaking SRP if supported. Luckily it's supported by most languages either natively or with a library now. | |
| Mar 17, 2015 at 23:43 | comment | added | Samuel | This is what we call Design pattern happy. Right? ;) | |
| Mar 13, 2015 at 8:43 | comment | added | Laurent LA RIZZA |
@Aitech: Effectively, it just means that ITenantStore is not a stable abstraction. It effectively depends on TenantStore, even though you applied DIP.
|
|
| Mar 13, 2015 at 8:38 | comment | added | Laurent LA RIZZA |
@Aitch: It does. You have the same set of methods in both classes. Add a method in le implementation class, and you have to add one in the logger, or you lose one line of log. It's not a dependency that prevents you to compile, but a dependency nonetheless. My definition of dependencies is: *A depends on B whenever a change in B induces a change in A. The first change you'll make if you want to add functionality to TenantStore is add a method. You then have to publish it through the ITenantStore, forcing an implementation in TenantStoreLogger.
|
|
| Mar 13, 2015 at 0:17 | comment | added | Aitch | @LaurentLARIZZA that's completely wrong, since the logger does not depend on the store implementation. | |
| Mar 13, 2015 at 0:10 | comment | added | Aitch | @jpmc26 also the question about what logging means to you. I don't want to debug my application. I want to known what happens on a business level. Not every component is interesting to me like reading from a database which happens with every server request. | |
| Mar 13, 2015 at 0:04 | comment | added | Aitch |
@user949300 you didn't get the idea of decorating an ITenantStore, see the ctor TenantStoreLogger(ITenantStore tenantStore). why 2N classes?
|
|
| Mar 12, 2015 at 23:57 | comment | added | Aitch | @user2023861 I'm not familiar with C# but also Java EE allows DI to private attributes. just ignore it. it's the same as passing it as an ctor parameter. | |
| Mar 12, 2015 at 8:14 | comment | added | Laurent LA RIZZA |
Downvoted. You removed the logging concern from the Tenant class, but now your TenantStoreLogger will change every time TenantStore changes. You're not separating concerns any more than in the initial solution.
|
|
| Mar 12, 2015 at 6:47 | comment | added | jpmc26 | Doing this for every single class that needs logging drastically increases the complexity of your code base (unless so few classes have logging that you wouldn't even call it a cross cutting concern anymore). It ultimately makes the code less maintainable simply because of the large number of interfaces to maintain, which goes against everything the Single Responsibility Principle was created for. | |
| Mar 11, 2015 at 19:19 | comment | added | user949300 | Instead of TenantStore being DIPed with a general purpose Logger, which requires N+1 classes (when you add a LandlordStore, a FooStore, a BarStore, etc.) you have a TenantStoreLogger being DIPed with a TenantStore, a FooStoreLogger being DIPed with a FooStore, etc... requiring 2N classes. As far as I can tell, for zero benefit. When you want to do no-logging unit testing, you'll need to rejigger N classes, instead of just configuring a NullLogger. IMO, this is a very poor approach. | |
| Mar 11, 2015 at 18:05 | comment | added | user2023861 | I don't see you assigning the _logger variable anywhere. Were you planning on using constructor injection and just forgot? If so, you'd probably get a compiler warning. | |
| Mar 11, 2015 at 17:35 | vote | accept | Aitch | ||
| Mar 21, 2022 at 19:49 | |||||
| Mar 11, 2015 at 17:35 | vote | accept | Aitch | ||
| Mar 11, 2015 at 17:35 | |||||
| Mar 11, 2015 at 17:35 | comment | added | Aitch | Thank you for the nice link. Figure 1 on that page is actually what I would call my favorite solution. The list of cross cutting concerns (logging, caching, etc.) and a decorator pattern is the most generic solution and I'm happy not to be completely wrong with my thoughts, although the larger community would like to drop that abstraction and inline logging. | |
| Mar 11, 2015 at 16:32 | history | edited | z0mbi3 | CC BY-SA 3.0 |
edited body
|
| Mar 11, 2015 at 16:21 | history | edited | z0mbi3 | CC BY-SA 3.0 |
added 73 characters in body
|
| Mar 11, 2015 at 16:16 | history | answered | z0mbi3 | CC BY-SA 3.0 |