Comment by zimbatm | Hacker News Reader

Comment by zimbatm | Hacker News Reader

Parent Full thread

It feels like we're on totally separate universes.

I don't understand why you insist that shooting yourself in the foot is any argument. My goal is to convey a semantic to NOT have to read the code; obviously if we don't follow the same conventions it won't work.

I also don't understand how something being global is bad. A namespace is exactly that: global. It's like you would argue to not associate classes to constants because it makes them global. The issue comes when your class methods have side effects like Time.now or User.find because they make your tests harder to build. But that's not what I'm proposing.

Finally I understand the value proposition that you have regarding refactoring: if you change the implementation and keep the same interface then you don't need to touch the lines where the interface is used. But introducing a side effect rarely comes without parameters and because your lines look like SpamChecker.new(foo).spam? there is no way to introduce the new parameters without either changing the lines in question or using a global. As an example: using Akismet would require an API key. How do you introduce it without touching your interface ?

> It feels like we're on totally separate universes.

It does indeed. Never mind, it's happened before and will no doubt happen again. :-)

If all you're proposing is a coding convention, then I don't see why you're surprised/annoyed that the OP doesn't obey it; it's vanishingly rare. You're placing a significant structural constraint on your code that conveys only optional semantics, and ones that will apply to virtually no third-party code. Why not adopt a more rubyish convention and use bangs to denote methods that do have side-effects (c.f. String#gsub vs. String#gsub!)? This way you're not imposing any structural constraints on your code, only a minor naming restriction.

Re: refactoring, constructor params are only one possible change we might make. Our checker might store previously-seen content in an ivar, so that the first time it sees the content "hey sexy, why not call me?" it allows it to pass, but if it sees 1000 such messages in quick succession, it flags them as spam.

However: yes, changes to object instantiation will require changes to client code (although, as I pointed out earlier, it's easy to provide access to a default instance via a class method, providing both convenience and flexibility). In something other than a Rails application this would be mitigated using dependency injection, so that merely being a client of a particular object does not mean you have to know how to instantiate it. Unfortunately as Rails monopolises object construction for its own infernal purposes this isn't really possible, so you end up with unpalatable things like this knocking around all over the place:

    class GuestBookEntry < ActiveRecord::Base
      def spam_checker
        @spam_checker ||= UserContentSpamChecker.new(...)
      end
    end

While not ideal, this at least localises changes to one site in each client class, and one whose concern is directly related to the change you're making.

I don't think we'll have an agreement so I'll stop there :)

Every time I see something like SomeClass.new(val).some_method it makes me cringe. For me it's as if you would refactor Math.sqrt(2) to SquareRoot.new(2).value in case you want to change the implementation in the future.

Reply on news.ycombinator.com