2017-08-11 18 views
0

このコードをレールに書き込むより良い方法があるかどうかを知りたいと思います。条件がレールで失敗した場合はfalseを返します

def self.get_user_id(name) 
    current_user_id = User.current.id 
    user_id = User.where("name=?",name).id 
    admin = check_admin(current_user_id) 

    if (admin == TRUE || user_id == current_user_id) 
     istrue = user_id 
    else 
     istrue = FALSE 
    end 

    return istrue 
end 

答えて

0

改善できる点はほとんどありません。価値のある命名と戻り値あなたが成功した場合に具体的なuser_idを返すことができればうまくいかない場合はnilを返す方が良いでしょう。また、関数の最後に明示的にreturnと言う必要はありません。 Rubyには多くのコーディング標準がありますが、最良のことは良いコーディング標準に従うことです。だから、それは宣言して物を定義する特定の方法にあなたを拘束します。

def self.get_user_id(name) 
    current_user_id = User.current.id 
    user_id = User.where("name=?",name).id 
    admin = check_admin(current_user_id) 

    if (admin == TRUE || user_id == current_user_id) 
     return user_id 
    end 

    nil 
end 
0

それを書くのより慣用的な方法は次のようである:私が代わりにそのIDをユーザーオブジェクトを返すように変更

def self.get_user(name) 
    current_user = User.current 
    user = User.where("name = ?", name) 

    return nil unless user == current_user 
    return nil unless current_user.admin? 
    user 
end 

お知らせが、これはほとんどの時点で周りのIDを渡すよりも優れています。また、慣例により、「無」または「無効」にしたい場合は、falseの代わりにnilを返す方が良いでしょう。

このコードを採用するには、check_adminメソッドと基本的に同じadmin?というUserというインスタンスメソッドを実装する必要があります。これにより、あなたのコードはよりオブジェクト指向になります。これはRubyのより好ましいスタイルです。

関連する問題