2012-10-06 2 views
0

이 내 after_create 콜백입니다 : 그래서 지금 그 블록에after_create 콜백을 어떻게 리팩터링 할 수 있습니까?

after_create { |f| if f.target.class.eql?(Question) 
     if f.target.user != User.current_user 
      Notify.create_notify(Notify::QUESTION_FOLLOW, f.target.user, User.current_user, f.target) 
     end 
     elsif f.target.class.eql?(User) 
     Notify.create_notify(Notify::USER_FOLLOW, f.target, User.current_user,f.target) if f.target.can_mail_user(:follower) 
     end 
    } 

내가 이동 시도는 다음과 같습니다

after_create do |f| 
    if f.target.class.eql?(Question) 
    if f.target.user != User.current_user 
     Notify.create_notify(Notify::QUESTION_FOLLOW, f.target.user, User.current_user, f.target) 
    end 
    elsif f.target.class.eql?(User) 
    Notify.create_notify(Notify::USER_FOLLOW, f.target, User.current_user,f.target) if f.target.can_mail_user(:follower) 
    end 
end 
내가 그 코드를 개선하기 위해 할 수있는 다른 무엇

?

답변

3

가 대부분 Notify로 이동합니다. 이게 Follow 클래스라고 가정합니다. 리팩토링에서 스스로에게 물어볼 수있는 유용한 질문은 '이 클래스는이 동작에 대해 알아야합니까?'입니다.이 경우 대답은 '아니오'라고 생각합니다. 이 같은 일을보십시오 :

after_create do |f| 
    Notify.create_notify(f.target.user, User.current_user, f.target) 
end 

... 그리고 작업이 전송되는 알림 파악하는 것입니다에 통보 간다 분기 로직, 나머지

합니다.

0

개선하기위한 한 가지 방법은 is_a? 메소드를 사용하여 클래스 이름을 직접 비교하는 것이 좋습니다. 또한 변수 f는 설명이 충분하지 않습니다. 마지막으로, f.target을 저장하는 새로운 변수를 만들어서 반복하지 않도록하십시오. 예를 들어

:

after_create do |record| 
    target = record.target 
    if target.is_a?(Question) 
    if target.user != User.current_user 
     Notify.create_notify(Notify::QUESTION_FOLLOW, target.user, User.current_user, target) 
    end 
    elsif target.is_a?(User) 
    Notify.create_notify(Notify::USER_FOLLOW, target, User.current_user, target) if target.can_mail_user(:follower) 
    end 
end 
관련 문제