[Rails] Questions / Réponses

[Sephi-Chan]

A quoi sert le @user.build_hero ? Que fait-il ? Du code serait le bienvenu.

Le coup du render 'home/index' me semble à éviter. Dès lors que ta page d'accueil aura plus de complexité (et aura besoin de moult @variables), tu ne pourras décemment pas ajouter toutes ses dépendances dans ton action create. Je pense qu'une redirection est plus adaptée dans ce cas.

Pour ce qui est de la race, je pense que tu ne pas avoir besoin d'instancier une quelconque variable. Utiliser un attribut virtuel sur ton modèle me paraît plus pertinent. Mais pour t'

N'hésite pas à montrer le code de tes modèles et de tes vues. Je pense qu'on peut faire un truc aux petits oignons.

[Holy]

A quoi sert le @user.build_hero ? Que fait-il ? Du code serait le bienvenu.

Il me sert à accéder à mon objet Héros pour ma page d'inscription car j'ai un nested form pour la création du héros de l'utilisateur.

Alors, voici mes modèles :

class User < ActiveRecord::Base

  attr_accessible :contact_email, :email, :password, :password_confirmation, :persistence_token, :role, :hero, :hero_attributes



  has_one :hero, dependent: :destroy

  accepts_nested_attributes_for :hero



  email_regex = /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i



  validates :email, presence: true,

                    format: email_regex,

                    uniqueness: { case_sensitive: false }

                    

  validates :password, presence: true,

                       confirmation: true,

                       length: 6..40



  validates :password_confirmation, presence: true

end



class Hero < ActiveRecord::Base

  attr_accessible :firstname, :lastname, :nation, :race, :rank, ex

  belongs_to :user



  SEX = %w( Homme Femme Autre )



  validates :firstname, :format => { with: /^[a-záàâäãåçéèêëíìîïñóòôöõúùûüýÿ]+$/i },

                        :length => { within: 3..12 },

                        :presence => true,

                        :uniqueness => { case_sensitive: false, scope: :lastname }



  validates :lastname, :format => { with: /^[a-záàâäãåçéèêëíìîïñóòôöõúùûüýÿ\x20'-]+$/i },

                       :length => { within: 3..18 },

                       :if => lambda { |hero| !hero.lastname.blank? }



  validates :nation, :presence => true,

                     :inclusion => { in: Nation::NAMES }



  validates :race, :presence => true,

                   :inclusion => { in: lambda do |hero|

                     Race::NAMES[hero.nation] ||= []

                     Race::NAMES[hero.nation]

                   end }



  validates ex, :presence => true,

                  :inclusion => { in: self::SEX }

end



class Race

  NAMES = {

    'klum'    => %w( dwarf gwanling human ),

    'garn'    => %w( orc voda_orc gobelin ),

    'ulysk'   => %w( sylvan_elf dwarf human ),

    'ludwynn' => %w( high_elf night_elf lowland_human )

  }

end



class Nation

  NAMES = %w( klum garn ulysk ludwynn )

end

Ma vue "new" :

= simple_form_for @user, :url => register_path, :validate => true do |u|

  .observator

    = t('observator.register.introduction')



  .col-50

    %h2= t('hero')



    = u.simple_fields_for :hero do |h|

      = h.input :firstname



      = h.input :lastname



      = h.input :nation, :collection => Nation::NAMES.map { |name| [ t(name, scope: 'nations'), name ] }



      = h.input :race, :collection => @race.map { |name| [ t(name, scope: 'races'), name ] }



      = h.input ex, :collection => Hero::SEX



  .col-50

    %h2= t('avatar')

    

    .avatar

      = image_tag 'avatar/sample.png'



    %h2= t('connexion')



    = u.input :email



    = u.input :password



    = u.input :password_confirmation



  .col-100

    .field

      = raw t('rules_agreement', :rules_link => link_to(t('rules_link'), rules_path))



    = u.submit t('title.register')

Le coup du render 'home/index' me semble à éviter. Dès lors que ta page d'accueil aura plus de complexité (et aura besoin de moult @variables), tu ne pourras décemment pas ajouter toutes ses dépendances dans ton action create. Je pense qu'une redirection est plus adaptée dans ce cas.

C'était une solution provisoire, mais ça recoupe en partie les problèmes d'instanciation ^^

[Sephi-Chan]

Tu pourrais remplacer le :

= h.input :race, :collection => @race.map { |name| [ t(name, scope: 'races'), name ] }

par quelque chose commme (à la louche) :

= h.input :race, :collection => Race::NAMES[h.object.nation] }

[Holy]

Tu pourrais remplacer le :

= h.input :race, :collection => @race.map { |name| [ t(name, scope: 'races'), name ] }

par quelque chose commme (à la louche) :

= h.input :race, :collection => Race::NAMES[h.object.nation] }

Comme j'étais pas trop sûr de ce que tu voulais dire dans ta réponse (y a un "}" qui traîne), j'ai essayé deux solutions :

= h.input :race, :collection => Race::NAMES[h.object.nation].map { |name| [ t(name, scope: 'races'), name ] } 

# => undefined method `map' for nil:NilClass



= h.input :race, :collection => Race::NAMES[h.object.nation]

# => Retourne un champ texte classique et non un select

L'erreur renvoyée par la première ligne me semble logique dans la mesure où h.object.nation ne renvoie rien (j'ai indiqué nulle part de valeur par défaut, ni dans ma base de données, ni dans mon modèle).

[Sephi-Chan]

= h.input :race, :collection => Race::NAMES[h.object.nation || 'klum' ]

Comme ça, quand le héros n'a pas de nation, on lui suggère les races liées à la nation Klum.

[Holy]

= h.input :race, :collection => Race::NAMES[h.object.nation || 'klum' ]

Comme ça, quand le héros n'a pas de nation, on lui suggère les races liées à la nation Klum.

Nice ^^

J'aime beaucoup ce type de notation, c'est pratique et clair à la fois. J'imagine qu'on peut cumuler ce type de décalage (j'aurais pu ajouter || 'Hey Oh du bateau' derrière comme ultime choix) ?

J'ai une dernière question que je vais présenter sous forme d'hypothèse pour l'organisation de mon contrôleur :

class UsersController < ApplicationController

  def new

    @title = 'register'

    if request.post?

      @user = User.new(params[:user])

      redirect_to 'home/index' if @user.save

    else

      @user = User.new

      @user.build_hero

    end

  end

  # Je pourrais supprimer ma méthode "create" de cette façon, tout en conservant mon code lisible et en respectant le principe de DRY.

end

Ca vous semble correcte comme approche ?

[Sephi-Chan]

C'est naze. Pourquoi rendre complexe ce qui est simple ? :p

[Holy]

Ben pour moi c'est tout aussi simple.

Entre ça :

class UsersController < ApplicationController

  def new

    @title = 'register'

    if request.post?

      @user = User.new(params[:user])

      

      redirect_to 'home/index' if @user.save

    else

      @user = User.new

      @user.build_hero

    end

  end

end

Et ça :

class UsersController < ApplicationController

  def new

    @title = 'register'

    @user = User.new

    @user.build_hero

  end



  def create

    @user = User.new(params[:user])



    if @user.save

      redirect_to 'home/index'

    else

      @title = 'register'



      render 'new'

    end

  end

end

Je trouve la première plus logique : une vue par action du controlleur. Bon après c'est une question de goût, mais c'est si choquant que ça comme approche ? XD

[Sephi-Chan]

C'est pas plus simple en terme de code (dans ce cas précis), mais plus simple en terme de concept :

• Tu sais que l'action new est toujours appelée en GET, sans paramètre.
  
• Tu sais que l'action create est toujours appelée en POST, avec des paramètres pour créer un utilisateur.
  

Comme ça, les ramifications sont plus simples. A chacun sa responsabilité, et les hippopotames seront bien gardés !

Dans le code, les if sont des sources de complexité qu'il faut limiter.


À côté de ça, tu pourrais virer le titre qui est une responsabilité de la vue.

class UsersController < ApplicationController

  def new

    @user = User.new

    @user.build_hero

  end

 

  def create

    if @user = User.create(params[:user])

      redirect_to root_path

    else

      render 'new'

    end

  end

end

# app/views/users/new.html.haml

- content_for :title, t('users.sign_up')





# app/views/layouts/application.html.haml

%title

  Mon jeu qui poutre —

  = yield(:title) || t('application.default_title')

Et au final, c'est plus propre à tous les niveaux.

Qu'en penses-tu ?

[niahoo]

Les @ c'est bien des variables d'instance ?