Repetitive controller with all methods nearly identical

Question

I have an API with the following code

 class API::V1::ReceptionController < API::V1::APIController before_filter :ensure_document_exists def produto jid = ProdutoWorker.perform_async params[:document] render json: {jid: jid}, :status => 200 end def cliente jid = ClienteFornecedorWorker.perform_async params[:document] render json: {jid: jid}, :status => 200 end def nota_fiscal jid = NotaFiscalWorker.perform_async params[:document] render json: {jid: jid}, :status => 200 end def venda jid = VendaWorker.perform_async params[:document] render json: {jid: jid}, :status => 200 end def reducaoz jid = ReducaoZWorker.perform_async params[:document] render json: {jid: jid}, :status => 200 end def impressora_fiscal jid = ImpressoraFiscalWorker.perform_async params[:document] render json: {jid: jid}, :status => 200 end ..............

and so on. What happens is: every method call a different worker, but all the body's methods are pretty much the same. My question is: how can I reduce this methods to maybe just one?

Can you show what ProdutoWorker means? As your code is currently missing context. — skiwi
– skiwi, Commented Jul 23, 2014 at 12:01
yes, of course! params[:document] will always be a JSON that represents a model (a product, a client, an order and so) any of these SomethingWorker receives this JSON and process him following some internal rules — Luiz K.
– Luiz K., Commented Jul 23, 2014 at 12:05

tokland · Accepted Answer · 2014-07-23 12:46:13Z

Some notes:

I don't know if there is an agreement on the community on that, but I would definitely use the de-facto language of the computer industry (translating the API routes if needed, of course).
I'd write parens on "normal" (non-DSL) method calls.
That's a lot of actions. Why don't you refactor it to have a unique action with a param?

I'd write:

class API::V1::ReceptionController < API::V1::APIController before_filter :ensure_document_exists WorkerClasses = {:produto => ProdutoWorker, ....} def process worker_class = WorkerClasses[params[:key].to_sym] if worker_class jid = worker_class.perform_async(params[:document]) render(json: {jid: jid}, :status => 200) else render(json: {errors: "Unknown key: #{params[:key]}"}, :status => 400) end end

(and change the routes accordingly, of course)

skiwi · Accepted Answer · 2014-07-23 12:07:29Z

I'm not familiar with Ruby, so I will write a no-code answer here, discussing what you could change.

I am assuming that ProdutoWorker, etc. all extend a Worker class, where extending is an OOP-definition here.

Then you should make a method that takes the Worker as input, and then calls something along the lines of:

jid = worker.perform_async params[:document] render json: {jid: jid}, :status => 200

where worker is the input.

Then you can save both one line per method definition you currently have, and more importantly you abstract away the logic into your helper method, so you cannot make a mistake in the logic of one of your implementations.

Johnson · Accepted Answer · 2014-08-12 18:06:55Z

What I would do is.

before_filter :render_json def render_json render json: {jid: jid}, :status => 200 end

And then remove that line from all your controller actions.

Stack Exchange Network

Repetitive controller with all methods nearly identical

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Repetitive controller with all methods nearly identical

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions