Live data from Hacker News

Rails – The Missing Parts

eng.joingrouper.com

151–160 of 173 posts

Re: Rails – The Missing Parts

#151

Earlier quoted context omitted.

I've only been involved in a couple of Rails projects (at least since the wedding list management app I wrote for my own wedding back in the v1 days), I'm in complete agreement with David. And honestly, Rails already has something that works beautifully for an “interactor” extraction. It's called ActiveModel. Add a bit of other code (include ActiveModel::ForbiddenAttributesProtection, ActiveModel::Validations::Callba…

> And honestly, Rails already has something that works beautifully for an “interactor” extraction. It's called ActiveModel. The thing which Rails and DHH offer no guidance whatsoever is the separation of persistence from the domain model. The Active Record pattern is itself a conflation of those two things, so this is an opinionated choice, and I respect it as a sane default for a wide variety of applications. It wor…

Here's the thing, though: Rails purposefully doesn't provide more than the minimum. I disagree with DHH on many things (ActiveRecord’s persistence model being based on MySQL misfeatures has been one of my biggest frustrations with Rails since he introduced it at a RubyConf where we both presented), but he's right that this sort of thing doesn't belong in Rails.

Everyone[1] needs a persistence layer and domain objects. That ActiveRecord is both ActiveModel and persistence is neither here nor there—and Rails 3 did a lot to making it possible to separate ActiveModel (domain object behaviour) and ActiveRecord (persistence layer behaviour) in a logical perspective so you can choose to persist differently than ActiveRecord does. Everyone needs some kind of controller. Everyone needs some kind of presentation layer. Aside from providing sane defaults for all of this, Rails doesn't try to dictate how your application is built.

The “more complex” architecture required for your application is unlikely to be a good fit for my application’s “more complex” architecture. For Rails to anoint one particular “more complex” architecture…is to fail anyone else who doesn’t need that architecture or would need something different. If you want to promote a particular type of architecture, there are ways: write an Engine; include generators. Give people guidance. Tell people why they want to use your particular architecture, but also tell them why they shouldn’t use it. If a lot of people need it, they will adopt it. Like Rspec. Like cucumber. Like ActiveModel::Serializers.

And, honestly, consider whether you really need your “more complex“ architecture or whether you can combat it the same way that Basecamp does. We're doing that with our product team, trying to find features we can remove and reimplement later when we really need them. Ours is a fairly mature product, so it's hard—but it's happening.

[1] For some large subset of everyone. Anyone who is reaching for something like Rails, Django, etc. needs some sort of persistence layer. Anyone who needs to store data needs that. Everyone needs domain objects.

Re: Rails – The Missing Parts

#152

Earlier quoted context omitted.

> And honestly, Rails already has something that works beautifully for an “interactor” extraction. It's called ActiveModel. The thing which Rails and DHH offer no guidance whatsoever is the separation of persistence from the domain model. The Active Record pattern is itself a conflation of those two things, so this is an opinionated choice, and I respect it as a sane default for a wide variety of applications. It wor…

Here's the thing, though: Rails purposefully doesn't provide more than the minimum. I disagree with DHH on many things (ActiveRecord’s persistence model being based on MySQL misfeatures has been one of my biggest frustrations with Rails since he introduced it at a RubyConf where we both presented), but he's right that this sort of thing doesn't belong in Rails. Everyone [1] needs a persistence layer and domain object…

Where did I say Rails should provide this? My point was primarily that attempting to convince DHH of your architectural choices is sisyphean and ultimately pointless since he has crafted a nice bubble for himself where he doesn't have to be exposed to architectures or complexities that he finds distasteful.

Re: Rails – The Missing Parts

#153
post #54

Earlier quoted context omitted.

Yes, rewriting a system after it's already settled and designed can indeed bring a cleaner code base about. But don't confuse that with "better architecture". Don't even get me started on the hand-wavy "loosely coupled". If this is a poor example, pick a good example. I'll be happy to code ping pong you whatever example you choose. One way to spend hundreds of thousands of LOC on an application is to stuff it with ne…

I hope I am not sticking my head into the lion's mouth here, but here ( https://gist.github.com/BiggerNoise/9334673 ) is a link to a gist of some code in one of our apps. Cases have Tasks and Activities. Activities are always associated with a case, and may be associated with a task. In the controller, when assigning a case, one would type: AssignCaseCommand.new(params).execute! The command class implements the Comma…

I asked for specific code, you gave me specific code, so here's how I would have structured that without the (imo, needless) command pattern: https://gist.github.com/dhh/9348053

It includes a lot of more context than your example, so I was guessing liberally at the missing pieces. But hopefully this is still easily digestible.

Re: Rails – The Missing Parts

#154

Earlier quoted context omitted.

Regardless of reuse, controller actions can get awfully large if unchecked. What would you say is the maximum LOCs for a public controller method? Callbacks in Controllers and AR::Models tend to make things worse IMO for anything other than authentication; and for intricate actions interactors seem like the best defence. I have had the (mis)fortune to jump into a number of large Rails codebases and I can say with han…

> What would you say is the maximum LOCs for a public controller method? As small as it needs to be in order to get the job done, and as large as it must be to get it done clearly. Sometimes that's zero lines of code; sometimes it's 500. You can usually factor down a 500 line method, but it may be worth asking what the breadth cost is should the code really be single-use. If your metric is anything else, then you're…

You're really against Code Climate huh? Just what did it do to you to garner such hatred? I'm not personally a fan but I can see how it may help large companies in certain situations.

Anyways, when I asked about a "maximum" I was not asking in absolute terms. What do you see as a reasonable max for the majority of use cases?

At which point do you start feeling bad about your method?

Re: Rails – The Missing Parts

#155

Earlier quoted context omitted.

Here's the thing, though: Rails purposefully doesn't provide more than the minimum. I disagree with DHH on many things (ActiveRecord’s persistence model being based on MySQL misfeatures has been one of my biggest frustrations with Rails since he introduced it at a RubyConf where we both presented), but he's right that this sort of thing doesn't belong in Rails. Everyone [1] needs a persistence layer and domain object…

Where did I say Rails should provide this? My point was primarily that attempting to convince DHH of your architectural choices is sisyphean and ultimately pointless since he has crafted a nice bubble for himself where he doesn't have to be exposed to architectures or complexities that he finds distasteful.

> The thing which Rails and DHH offer no guidance whatsoever is the separation of persistence from the domain model.

This seems to suggest you were looking for guidance or sane defaults for this. If that's not what you intended it to mean, I'm sorry for misreading it.

Re: Rails – The Missing Parts

#156

Earlier quoted context omitted.

> What would you say is the maximum LOCs for a public controller method? As small as it needs to be in order to get the job done, and as large as it must be to get it done clearly. Sometimes that's zero lines of code; sometimes it's 500. You can usually factor down a 500 line method, but it may be worth asking what the breadth cost is should the code really be single-use. If your metric is anything else, then you're…

You're really against Code Climate huh? Just what did it do to you to garner such hatred? I'm not personally a fan but I can see how it may help large companies in certain situations. Anyways, when I asked about a "maximum" I was not asking in absolute terms. What do you see as a reasonable max for the majority of use cases? At which point do you start feeling bad about your method?

I'm against Code Climate because it promulgates nonsensical metrics in ways that are actively harmful to good engineering. Just like Bob Martin and his concept of “clean code”. All of these things are at best sigil markers. They aren't solutions, and they can't provide any answers.

I don't have a max length to a method, because I'm not a prescriptivist. What I have is a point where I stop understanding what a method is doing, or I can't explain it cleanly. Sometimes I keep the method as long, but better comment it. Sometimes I factor it out. The point at which I do so varies based on the code, time, and many other factors.

I've worked on a C++ codebase that had a 3,000+ line function that was nearly impossible to factor out because of a non-trivial amount of state that would have either:

a. required passing much of that state as parameters to the factored out functions/methods (even if you put it in a wrapper object/struct); or

b. extracting all of the (working) behaviour into one or more objects whose sole purpose is to encapsulate the behaviour of this function.

I absolutely hated working on that function, but it was essentially the main loop of the program. You could step through those 3k lines and get a fairly decent feel for what the program was going to do, when, and how often it would repeat. The hardest part was where people before had extracted code out…that was called exactly once. We tried three times to extract the code—and failed three times, ending up leaving the code the way it was because our extractions were more complex and less understandable than the existing crappy code.

My point is that there's no such thing as a reasonable max for the majority of use cases. It depends on what you're doing and in what code.

Re: Rails – The Missing Parts

#157
post #134

Earlier quoted context omitted.

No. I was not arguing anything about it, but the fact that it is a proper abstraction. The persistence is part of the domain. You need to know about when your objects are persisted and when they are not, this can be an intrinsic part of some of your business logic. Thus the events are relevant to your domain. Of course not all your business logic needs to be aware of persistence, and nowhere do you see me arguing tha…

Fair enough—I was being a bit of an asshole. However, when you start your contribution to the thread by insinuating DHH is above reproach, I'm not filled with sympathy. Especially when his architectural hipsterism is IMHO responsible for ruining a couple microgenerations of developers who learned to code through rails. I can't be the first guy who's put his head through a wall upon hearing you say persistence is part…

To be fair, a great many of web apps can justifiably say "my domain is the data store." In fact, whenever I use BaseCamp, that's exactly what strikes me almost immediately. Stock rails tends to be a great fit in those cases; your business objects are bags of data. Of course, those projects don't happen to be all that interesting to work on technologically, but I wouldn't want to work on MS Access "apps," either.

Re: Rails – The Missing Parts

#158
post #120

Earlier quoted context omitted.

I think it's great that you stick your head into the lion's mouth, even if he chewed it clean off it's a great contribution :) I think the main problem with your command pattern approach is that you didn't implement the full command pattern. At least, I assume you didn't, you didn't actually show the controller. The command pattern has one extra class, the invoker. The invoker takes commands, and invokes them. A grea…

Honestly, I was disappointed in his response. With all his harrumphing about real examples, I would have expected him to make an effort to understand a real example. But, as I said elsewhere, the downside of real examples is that they require quite a bit more effort to grasp than contrived ones. To address your comments: While it is true that one of the great features (and frequent motivations) for using the Command…

The Command pattern always reminds me of free monads. By building a new free monad you can encode the structure and logic of a computation statically and then evaluate it repeatedly. One of the things that I think is missing with the command pattern is that it's probably not as easy to decide to, for instance, execute your commands in a pure environment.

This is, of course, a primary advantage of having a static representation of the domain-level demands of your code—you can choose many ways and many environments to execute it within. It's a function I use in real code all the time.

Re: Rails – The Missing Parts

#159

Earlier quoted context omitted.

Do you have an example of a public controller method larger than even 25 lines that does its job clearly?

Controller method that I can share? No. This method in mime-types is reported by Code Climate as a code smell. They're wrong: it's the smallest it can possibly be while still correctly performing the necessary goal; any smaller, and you have to break it into multiple smaller methods that provide no value except keeping Code Climate happy. https://github.com/halostatue/mime-types/blob/master/lib/mim... The method is ~…

> They're wrong: it's the smallest it can possibly be while still correctly performing the necessary goal; any smaller, and you have to break it into multiple smaller methods that provide no value except keeping Code Climate happy.

Wrong? Smallest it can possibly be? Those are strong claims.

I definitely disagree with Code Climate from time to time. But I try to keep an open mind.

I've found that trying to follow dumb rules can be enlightening, even when my first reaction is that I disagree. This goes for Code Climate as well as arbitrary rules, such as Sandi Metz': https://gist.github.com/henrik/4509394

I looked at your `priority_compare`, for example. As someone who did not write that code, I find it a little hard to follow. It has conditionals four levels deep (counting the ternaries). Your `Mime::TYPE` class is quite long, as is its test.

I would let the Code Climate feedback inspire me to attempt to extract a method object in this case, along these lines: https://gist.github.com/henrik/9355101

That would result in more code overall, but smaller objects on average, and to my eyes readability is improved because the complexity is a little abstracted. Do you truly feel it provides no value?

Re: Rails – The Missing Parts

#160

Earlier quoted context omitted.

Do you have an example of a public controller method larger than even 25 lines that does its job clearly?

Controller method that I can share? No. This method in mime-types is reported by Code Climate as a code smell. They're wrong: it's the smallest it can possibly be while still correctly performing the necessary goal; any smaller, and you have to break it into multiple smaller methods that provide no value except keeping Code Climate happy. https://github.com/halostatue/mime-types/blob/master/lib/mim... The method is ~…

Also glanced at the load_from_v1 code.

I can't argue with the "deprecated code" bit – if you never need to understand or edit that code again, by all means leave it. There will be no payoff for spending time reducing the complexity.

But "splitting into multiple methods only complicates the logic flow" sounds less like Code Climate being wrong and more like you not having found good refactoring techniques, maybe.

I see several things that I think have a good chance of helping readability: extracting the whole thing to an object, then replacing the inline error handling with an error handling method ("begin" is a smell), extracting (and naming) whatever happens on each iteration of the "data" and so on.

Post reply on HN