CODEWITHSUNDEEP
rubyruby-on-rails-5
Leron
Leron

Reputation: 9866

Use the result of checked method inside conditional statement

I'm trying to implement and Authorization module, because currently the authorization logic for a certain resource is separated on two or three different places and even though I'm not sure this is the best approach, at least I think it will provide some encapsulation.

Since I'm checking for several different things

  1. Does the user have the right role
  2. Is the resources in the right state to process the required action
  3. Do the user has the right to perform the required action on this particular resource

So as you can see, there are several checks, I'm not pretending to be completely correct here, but this is pretty close to the real case, so I've decided to use something like a Result Object even though it's actually not an object but a struct and I'm not using Gem but pretty simple custom implementation.

So part of my Authorization module is this:

module Authorization
  Result = Struct.new(:successfull?, :error)

  extend self

  def read(user, resource, message: 'Permission denied')
    can_read =
      [
        condition1,
        condition2,
        condition3
      ]

    return Result.new(can_read.any?, can_read.any? ? nil : message))
  end

However within this Authorization module I have a lot of methods and some of them check read internally like so:

def assign(user, resource, message: 'Permission denied')
  return read(user, resource) unless read(user, resource).successfull?

  Result.new(true, nil)
end

So my main question is how to avoid this double call to read(user, resource). I guess one option would be to just call it before the check like:

result = read(user, resource)
return result unless result.successfull?

However I'm pretty new to Ruby and I suspect that maybe there is more ruby-like way to do this. Just to inline it somehow by assigning the result from read inside the condition check...However this is just wild guess.

And one more question, that came up while I was writing this. Currently if I want to send nil for message when the authorization passes I'm doing this:

return Result.new(can_read.any?, can_read.any? ? nil : message))

Because message unless can_read.any? is throwing and error even though I thought it would default to nil. So again, is there some more ruby-like way to do this?

Upvotes: 1

Views: 48

Answers (1)

Marcin Kołodziej
Marcin Kołodziej

Reputation: 5313

First part can be written with Object#yield_self:

def assign(user, resource, message: 'Permission denied')
  read(user, resource).yield_self do |res|
    res.successful? ? Result.new(true, nil) : res
  end
end

successfull? -> successful? for English reasons. I am not convinced this is more readable than using a local variable though. Alternatively:

(res = read(user, resource)).successful? ? Result.new(true, nil) : res

As for your second question, you'll need more parentheses

Result.new(can_read.any?, (message if can_read.none?))

the return is not needed.

I would also advise you to slow down with all the unlesses, try to swap your conditions to if whenever possible -- I find it quite useful to make Result a class and define a failed? method for it. Actually, I'd consider this:

class Result
  def initialize(error)
    @error = error
  end

  def successful?
    @error.nil?
  end

  def failed?
    !successful?
  end
end

That depends on how complicated your Result gets, but for the use case shown, it would be a little cleaner imho.

Upvotes: 2

Related Questions