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

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).

nataz dit

le Mercredi 22 avril 2009 à 06h06

Concernant les valeurs variables de bouton, j'ai une solution simple (d'ailleurs je n'ai pas trouvé mieux pour faire mes localisations) :

séparer totalement la structure des valeurs

Ce qui implique différents points :

  • chaque champs a un id unique (ex. titleMister, MobilePhone, etc.)
  • chargement d'une localisation lors de la création de l'objet/application (depuis la DB ou en include) suivant la langue de l'utilisateur sous forme d'un tableau "MobilePhone" => "télephone mobile", etc.
  • les clés du tableau sont utilisées dans la structure des formulaires (ou autre) et les valeurs pour l'affichage : le champ MobilePhone sera toujours le même, seul le label associé change

Cela fonctionne bien (j'ai pu ajouter à la volée des langues à des applications relativement complexes sans jamais toucher au code, juste en insérant les données dans une table de DB). Il faut juste gérer les cas de traductions manquantes par l'ajout d'un message dans le label concerné.

nataz dit

le Mercredi 22 avril 2009 à 06h06

arf, la syntaxe markdown à viré la phrase principale de mon commentaire :/

en troisième ligne il faudrait lire:

«Toujours séparer la structure du contenu»

Martin Catty dit

le Mercredi 22 avril 2009 à 09h09

Ça va encore, je m'attendais à pire. Le coup du si tu veux retourner à un endroit spécifique je te retourne à la home est bien sympa.

Par contre pour les sauvegardes d'objets imbriqués il existe une solution bien pratique qu'on ne voit pas souvent en place c'est le validates_associated.

Tu peux par exemple avoir une table adresse distincte parce que ton utilisateur peut avoir une adresse mais aussi une société et ajouter le validates_associated dans ton utilisateur (couplé à un validates_presence_of).

Si l'adresse ne remplit pas les critères de validation définit dans le modèle adresse, ton utilisateur ne sera pas sauvé. Ça évite les gestions manuelles de valid? et aussi de toucher à ta modélisation.

Meuble dit

le Mercredi 22 avril 2009 à 10h10

nataz > Ouai, c'est une bonne solution. Un autre un peu plus simple c'est que l'id unique soit la clé de traduction, pas la traduction elle même.

Martin > Validates_associated c'est bien, quand il y en a besoin. Dans le projet en question, il n'y en a pas besoin du tout : un utilisateur n'a qu'une adresse et aucun autre modèle n'a d'adresse. Autant dénormaliser.

Ah, et oz m'a fait remarquer que j'avais fait un lapsus en début de dernier paragraphe : j'ai écrit démoralisait au lieu de dénormalisait. Révélateur ?

nataz dit

le Mercredi 22 avril 2009 à 13h01

meuble > absolument; c'est ce que j'ai voulu dire mais apparemment pas assez clairement ;)

Meuble dit

le Jeudi 23 avril 2009 à 15h03

nataz > Autant pour moi alors ^_^