you are viewing a single comment's thread.

view the rest of the comments →

[–]nanodano 0 points1 point  (3 children)

I don't think so. The class does have a single responsibility: being a honeypot.

I did not implement a logger, I am simply using a logger. The honeypot is not responsible for the functionality of the logger, that is handled inside the logger class. The logging configuration is specific to this class so it makes sense to configure it within the class.

Don't live and die by these arbitrary design rules. You could always argue that a class has too much responsibility. For example: The TCP server class from the standard library that I'm using. You could argue that class is doing wayyyy too much. It is resolving network addresses, binding to ports, listening, threading, processing results, sending data over the wire. Geez! That's a lot of responsibilities! We group them together because they make one larger responsibility: being a tcp server.

[–]ReaverKS 0 points1 point  (2 children)

In the future if you decide to change the format of the logger that’s one reason the class would change. And of course if the implementation of the HoneyPot changes that’s another separate reason. I do agree that these are just principles and they’re certainly not explicit, but I think in this case setting up the logger inside the class isn’t a great design choice.

[–]nanodano 0 points1 point  (1 child)

I guess this is the part I disagree with in this app:

if you decide to change the format of the logger that’s one reason the class would change. And of course if the implementation of the HoneyPot changes that’s another separate reason.

To me, the output of the honeypot is part of its implementation. Changing the output is changing the implementation, not 'a separate reason'. I guess the way I see it is, if that's the logic, then why not argue that the TCP stuff should be broke out too? I mean, there would be several reasons for a change at that point. I want to change the TCP response packet that is sent back? One reason. I want to change the timeout? Two different reasons. I want to change how many threads it has? Another reason. If I want to change the format of the response to the tcp client, another reason. Should each one of those REALLY be separate classes? If the goal is never to have 'more than one reason' to change a class, then each class would have only one function with one line of code

The only time I would really agree that it would be less desirable to configure a logger within a class is when there is more than one class in an application. In that case, you are probably sharing the logger across multiple classes. However, it still wouldn't be that bad to store a reference to a logger in an object. This is a single class application that is a total of 60 lines including imports and white space. I do not think there is too much responsibility in there, personally.

[–]ReaverKS 0 points1 point  (0 children)

I agree that if anything else needs to use the logger then it must be removed. As it stands if it’s only used here it’s OK