Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorating nil object #36

Merged
merged 2 commits into from
Dec 23, 2023
Merged

Decorating nil object #36

merged 2 commits into from
Dec 23, 2023

Conversation

kalashnikovisme
Copy link
Contributor

Discussion

What should we do with nil-object decorating? There are 3 approaches:

  1. Return just nil
  2. Decorate nil object
  3. Raise error

I've implemented the first approach here. Waiting for your opinion 🙂

What's changed for tramway drivers?

Before

user = nil
UserDecorator.decorate user # => UserDecorator#object

After

user = nil
UserDecorator.decorate user # => nil

@haukot
Copy link

haukot commented Dec 18, 2023

I think 1 is okay, but it would be better to check how popular decoration libraries (e.g., draper) are working, and whether people are raising issues about having problems with them.

Probably, it would be good if this behavior could be easily redefined in inherited decorators.

@kalashnikovisme
Copy link
Contributor Author

@haukot what about options for this thing, of course, it will be implemented later.

@moshinaan
Copy link
Collaborator

@kalashnikovisme in which cases u can get nil for decorating?

@kalashnikovisme
Copy link
Contributor Author

@moshinaan example:

class Post < ActiveRecord::Base
  belongs_to :user, optional: true
end
class PostDecorator < Tramway::BaseDecorator
  def user
    UserDecorator.decorate object.user
  end
end

in the future, there will be something like this

class PostDecorator < Tramway::BaseDecorator
  belongs_to :user, decorator: UserDecorator
end

So, this case takes its place 🙂

@kalashnikovisme
Copy link
Contributor Author

kalashnikovisme commented Dec 23, 2023

What about draper working with nil objects?

Draper uses option 2 - decorating nil objects, and I don't like it in the case of decorating ActiveRecord models. I think that replacing ActiveRecord object with a decorated object should be inconspicuous in controllers. So, the behavior of decorated objects should be similar to the ActiveRecord object as much as possible.

What about active_decorator working with nil objects?

ActiveDecorator uses option 1. But it's not an example, because ActiveDecorator decorates all ActiveRecord objects by default. But if ActiveRecord object is nil, the decorated object is nil too (or maybe decoration does not happen).

@haukot what do you think?

@kalashnikovisme kalashnikovisme merged commit ac0e9dc into main Dec 23, 2023
2 checks passed
@kalashnikovisme kalashnikovisme deleted the decorator_with_nil_object branch December 23, 2023 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants