Comment by daniel-levin | Hacker News Reader

Comment by daniel-levin | Hacker News Reader

I like the idea of one-class-one-responsibility but have a number of questions:

1) Isn't the whole idea of ActiveRecord that persistence is hidden from the model programmer? Surely the model doesn't actually have 'triple duty' because it doesn't contain any persistence code?

"Imagine you are a brand new developer on the team that supports this app. You see the @contact.save call but now, the fact that it performs business logic is even harder to see, since the logic is placed in another module, somewhere in some other directory." - I will imagine I am a new developer on the project:

2) My expectations have been subverted. My understanding of Rails conventions is that business logic does live inside the model which will be inside app/models. Therefore, the first question I'd ask myself would be: "What business logic happens when this an instance of this model is created?" Surely Rails supports this paradigm by virtue of the existence of after_create? Wouldn't I have to visit a third class in non-obvious location to understand the updated example(app/use_cases)?

3) Doesn't CreatesContact now have two responsibilities as well? Namely Creation and validation? And the name doesn't communicate that it performs validation as well, does it?

4) Doesn't creating this new class exacerbate the problem of code/logic being 'scattered all over the place'?

5.1) Doesn't the issue of: "[In the controller], To an outside observer, it is not entirely clear that an email will be sent." remain just as problematic as the model containing the code for sending the email? Because you'd have to mentally context switch (the single biggest obstacle to understanding code IMHO) from the controller to the CreatesContact class to see what is happening. when CreatesContact.new(@contact).create_contact! is called.

5.2) Continuing the thread of the last question, CreatesContact.new(@contact).create_contact! certainly doesn't say to the programmer: "I send an email if a certain flag was checked". Isn't this unintentionally self-defeating? The aim is a reduction of 'obfuscation', but isn't hiding what a 'single-responsibility' class actually does obfuscation itself?

I don't mean to be dismissive, but, broadly, I disagree with the way you have tried to compartmentalise this application. The purpose of my questions is to help me understand your opinion.

Hey, I'm not the OP, but I have some thoughts based on my experience creating the Obvious Architecture.

1) Hiding persistance from the programmer by tying it to your models is a bad idea. It's where a lot of problems start. First, how do you test without hitting the database? How long do your tests take? Minutes? Hours? Or do you skip them?

I don't think your models should do persistance at all. They should model your data for you. Do validation against your business rules. That sort of thing.

Would you tie your models to the filesystem? If not, then why would you tie them to the database?

Pulling your persistance mechanism out of your models is the first step to writing fast, maintainable, highly testable code, even if you are still using rails.

2) Rails conventions want you to tie your DB to your objects, which is a bad idea. Rails does a lot of things well, like it gives you a nice structure for controllers, views, routing, asset management, so use it for those things. Rails is a delivery mechanism. Use it as one and leave data modeling and persistance to something else.

3) I think the name issue could be solved by treating it as "action" object. Think of actions as more like actors. They do things. They integrate models/entities and persistance mechanisms along with minor amounts of logic that don't fit inside of entities.

As an action object, you could all it CreateContact and it would be called create_contact.rb in the app/actions folder. Then, when you look in the app/actions folder, the file name communicates what it does - it creates a contact.

4) Actually, it helps a lot with testing. Creating a new class makes dependency injection super easy, making testing super easy and fast. Also, it can help with clarity and glance factor. Imagine you have an app/actions folder with files like create_contact.rb, remove_contact.rb, send_email.rb, get_contact.rb, compose_email.rb and so on. You look at that folder and you can see that it is probably an contact management and email app. Maybe an contact or newsletter management app?

That kind of glance factor you can't get with a bunch of higher level controller or service containers. It also avoids the problem of "what controller/module/service does this method belong to?"

5) I actually agree that the code that the OP did was not the best in that the persistence and email sending mechanisms aren't passed in to the action, so that you don't know what it is going to try and save and where. If the code explicitly passed in the persistence and email mechanisms it would be more apparent that an email is going to be sent as part of the CreateContact action. That being said, I would probably make those two separate actions for clarity's sake.

I've written some apps using the Obvious Architecture and it really does help solve a lot of the problems that default Rails MVC creates.

Reply on news.ycombinator.com