palkan / rubanok

Parameters-based transformation DSL
MIT License
207 stars 8 forks source link

naming? #3

Closed jrochkind closed 5 years ago

jrochkind commented 5 years ago

I really like your approach here!

I do not like the "plane" and "planish" terminology. It doesn't mean anything, nor is it obviously connected to the name "rubanok" (unless you know Russian maybe!)

I think that if I write code using rubanok, future developers when they see a "SomethingPlane" class are going to be confused, and especially confused when they see a planish method in a controller. It's unclear what it does, and it's unclear it's related to Rubanok.

I would suggest either "query" or "transform(er)" instead as terminology. For the demonstrated use cases, a "query object" seems to describe really well what rubanok objects do. But rubanok may not be intended to be limited to or focused on "query objects", but to general transformations?

I also really like putting the name of the gem in any automatically-mixed-into-Rails methods that a gem adds. With some rails devs adding lots of gems, it can be very confusing to figure out where a method comes from, and the gem name on the auto-mixed-in-method makes it a lot clearer.

So maybe:

class CourseSessionsQuery < Rubanok::Query
  # ...
end

class CourseSessionController < ApplicationController
  def index
    @sessions = rubanok_query(CourseSession.all)
  end
end

Or

class CourseSessionsTransformer < Rubanok::Transformer
  # ...
end

class CourseSessionController < ApplicationController
  def index
    @sessions = rubanok_transform(CourseSession.all)
  end
end

(Or "Transformer" on the class name, but query on the controller-mixin-in, since in a controller that's pretty much what it's for?)

Something like this would make me a lot less reluctant to try out rubanok in a project. The existing naming just leaves me feel like I'm going to confuse my present and future colleagues.

I know I can easily insert my own code into Rubanok to use whatever I like... but when you can't easily google for "what's going on here" based on what you find in the code -- or if the docs and examples you find don't match what you found in the code, it's another thing that confuses developers. It's important to me that code that comes from gems be very clear about where it comes from and what it does, based on the naming and ability to google for that naming.

palkan commented 5 years ago

Thanks for sharing the feedback!

The existing naming just leaves me feel like I'm going to confuse my present and future colleagues.

That's a good point.

Let's find out a better naming!

I kinda like Rubanok::Transformer and transform but, IMO, it does not focus on all of the key things in Rubanok: take input and some params (or presets, maybe) and apply the transformations according to these params.

And that reminds me different kinds of processors (from ffmpeg to guitar pedals), so...

What about Rubanok::Processor? And process_query respectively (I agree that controllers deal with "queries" mostly).

Another idea is to call the base as simple as Rubanok::Base (which is a common practice). And it's up to the developer to choose the name for the ApplicationTransformer or ApplicationQueryProcessor or whatever.

The less opinionated the library is the better.

jrochkind commented 5 years ago

I kinda like Rubanok::Transformer and transform but, IMO, it does not focus on all of the key things in Rubanok: take input and some params (or presets, maybe) and apply the transformations according to these params.

To me, "transformer" does get that across. (But maybe I'm missing something, what do you mean by "input and params"? In the typical example, the controller "params" are the "input" no? In which cases are they different, and what do you mean by each?)

But "processor" seems reasonable to me too, if not as good (to me) as "transformer". Base as class name also works for me, although I wonder if Processor or Transformer is better to do some self-documenting as to what this thing is. But Base is not terrible.

process_query seems reasonable; personally I still like it when the gem name is in the method that is mixed in to a controller (otherwise you have to do some detective work to figure out where the method is coming from, if you come across a codebase you are unfamiliar with using a gem you are unfamliar with). The idea of putting the gem name in the method-name might not be universal though; but it is one I have come to from much experience.

palkan commented 5 years ago

OK, I'm back.

In the typical example, the controller "params" are the "input" no? In which cases are they different, and what do you mean by each?)

By "input" I mean an AR relation, for example. And by "params" controller params. So, we need both (though the latter one, "params", could be empty).

As for naming, I'm thinking of Rubanok::Processor (Base is not hat good if we would have some other features in the future) and rubanok_process with the ability to add aliases via the configuration (config.rubanok.controller_helper_alias = :rubanize).

jrochkind commented 5 years ago

processor/process seems an improvement to me!