home icon contact icon rss icon last FM icon facebook icon LinkedIn icon Delicious icon twitter icon

Archive pour le mot clé "best practices"

Cauchemars

Je suis loin d'être un maître de la programmation. Je suis sûr que mon code comporte lui aussi des trucs un peu moche. Mais quand même, des fois faut pas pousser.

Il y a des trucs qu'il ne faut pas faire.
Et il y a des gens pour les faire...

Redirection après login

if session[:return_to]
  redirect_to root_path
else
  redirect_to user_profile_url(@user_session.account.user)
end

Ca par exemple ça ne sert à rien. Pourquoi se préoccuper d'où vient l'utilisateur, pourquoi s'embêter à stocker ses volontés quand il se confronte à un accès restreint, si on n'en tiens pas compte ? Et si l'utilisateur ne voulait pas aller à l'accueil ?

Des boutons radio à valeurs variables

<label><%= t('title') %></label>
<span class="radio">
  <%= m.radio_button :title, t('mister') %>
  <%= m.label :title_mister, t('mister') %>
  <%= m.radio_button :title, t('madam') %>
  <%= m.label :title_madam, t('madam') %>
  <%= m.radio_button :title, t('miss') %>
  <%= m.label :title_miss, t('miss') %>
</span>

Vouloir internationaliser une application, c'est une bonne chose. Mais il y a des excès à ne pas atteindre. Si la valeur d'un bouton radio est internationalisé, la valeur sera différente en fonction de la langue de l'utilisateur. Déjà, seul comme ça ça peut être confusant.
Mais si en plus on enregistre le sex de l'utilisateur sans plus de contrôle, on aura en base dans le champs title : "Monsieur", "Madame" ou "Mademoiselle" mais aussi "Mister", "Mrs", "Miss", etc...

Imaginez alors qu'on veuille chercher tout les messieurs de notre application ? SELECT * FROM users WHERE title in ("Monsieur", "Mister"). Et si maintenant je rajoute une langue (au hasard, le russe), faut que je modifie toutes mes requêtes et que je rajoute господи́н.

Trop de normalisation, sans transaction

Sur une application, on a des membres. Ces membres sont de différents type, peuvent se connecter de différentes manières (classique, facebook connect, openId, ...), possèdent d'autres objets (articles, commentaires, projets) et ont une adresse et une image.

Si on normalise, on a un membre, qui est composé d'un user et d'un compte. Il a aussi une adresse postal (avec vile, code postal et pays) et ses articles. Et pour créer une user ça devient :

def create
  @account = Account.new(params[:account])
  @member = Member.new(params[:member])
  @visual = Visual.new(params[:visual])
  @address = Address.new(params[:address])
   if @account.valid? & @member.valid? & @visual.valid?
    @account.save
    @address.save(false)
    @member.create_user.account = @account
    @member.visual = @visual
    @member.address = @address
    @member.save

    flash[:success] = "Ok."
    redirect_to root_path
  else
    render :action => :new
  end
end

C'est long pour un contrôleur, non ? Admettons même que ce se soit dans un modèle, ça reste moche. Tout ces save dont certains sans validation, peuvent entraîner plein de problèmes :

  • Levée d'exception SQL parce qu'il n'y a pas les bons type
  • Il n'y a pas de transaction encadrant tout ça. Si ça ne marche pas à un endroit, on se retrouve avec une table bancale, voir corrompue.
  • Comme dans l'exemple on pourrait oublier de vérifier certains modèles (ici @adresse n'est pas validé et sauvé dans validation).
  • La modification d'un tel code n'est pas aisée et devant la confusion, on ne sait pas trop quel serait l'impact d'une modification...

Si au lieu de ça, on démoralisait un peu, on pourrait simplifier les choses. Un utilisateur n'a qu'une adresse postale, donc on peut ajouter les champs rue, ville pays et code postal au membre. De même, il n'a qu'un avatar, et même si d'autres modèle peuvent aussi avoir un avatar, il vaut mieux utiliser le duck typing et les surcharges d'opérateurs que la composition. Enfin, plutôt que de démultiplier les dépendances, on pourrait gérer les types de membre par de l'héritage (spécialisation) plutôt que par de la composition. Cela permet une plus grande généralisation (il y aura une méthode send_messagepour tout le monde, dont le comportement sera différent pour tous).