Yii Framework Forum: Code Bewerten - Yii Framework Forum

Jump to content

Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

Code Bewerten Rate Topic: -----

#1 User is offline   beat78 

  • Junior Member
  • Pip
  • Yii
  • Group: Members
  • Posts: 53
  • Joined: 13-May 11
  • Location:Zürich

Posted 08 May 2013 - 04:55 AM

Hallo Zusammen


Darf ich Euch fragen wie Ihr den Code findet? Mache ich das kompliziert, nicht korrekt oder ist das ok so?

Danke für das Feedback.

Posted Image


Controller
public function actionProject() {

        $data = new TblProjekt;
  $form = new TblStatus;
  $modelNachricht = new Tbl_Nachrichten;

        // Gast und $_GET ID prüfen 
        if (isset($_GET['id']) && !Yii::app()->user->isGuest) {

            $data = TblProjekt::model()->findAllByPk($_GET['id']);
   
        } else {
            $data = TblProjekt::model()->findAll();
        }
  
  // Wenn der Benutzer Gast ist und sich bewirbt
        if (Yii::app()->user->isGuest) {

            $msg = "Bitte zuerst anmelden.";
            $this->render('mobile', array('var1' => $msg, 'typ' => 'alert alert-error', 'data' => $data));  
   
        } else {
  
   if (isset($_POST['TblStatus'])) { 
  
    // Wenn der Benutzer angemeldet ist und sich bewirbt
    $form->setAttributes($_POST['TblStatus']);
    $modelNachricht->setAttributes($_POST['Tbl_Nachrichten']);
    $modelNachricht->idfs_User = Yii::app()->user->id;
    $modelNachricht->idfs_Projekt = $_GET['id'];
    $modelNachricht->save();
    $form->idfs_Bewerber = Yii::app()->user->id;
    $form->idfs_Faehigkeit = $form->idfs_Faehigkeit;
    $form->idfs_Projekt = $_GET['id'];
    $form->Datum = time();
    $form->Status = "Anfrage offen";
    $form->Save();
    
    
     if ($form->validate()) {
     
     // Mail versenden
     $Projekt = TblProjekt::model()->findByPK($_GET['id']);
     $User = User::model()->findByPK($Projekt->Benutzer);
     $AnbieterEmail = $User->username;
     Yii::app()->Mail->Send_Bewerbung($AnbieterEmail, $Projekt);
     
     $msg = "Die Bewerbung wurde erfolgreich an den Projektleiter versendet.";
     $data = TblProjekt::model()->findAll();
     $this->render('mobile', array('var1' => $msg, 'typ' => 'alert alert-success', 'data' => $data,'model'=>$form, 'modelNachricht'=>$modelNachricht));
     }
      
   } else {
   
   // Wenn Formular leer ist
   
   $msg = "Bitte wählen Sie die Fähigkeit aus";
   $this->render('mobile', array('var1' => $msg, 'typ' => 'alert alert-info', 'data' => $data,'model'=>$form, 'modelNachricht'=>$modelNachricht));
   }
    }
 }



View

<div> <?
  if (isset($var1)) {
   Echo "<div class='". $typ ."'>";
   echo $var1;
   echo "</div>";
  } ?>
 </div>

 <?
 
 // Wenn es um eine Bewerbung geht
 if( isset($model)) {
  
  // Pinboard anzeigen
  Yii::app()->Pin->CreatePin($data); 
  
  // Fähigkeiten auswählen
  $this->renderPartial('myform', array('model' => $model, 'modelNachricht' => $modelNachricht));  
  
 } else {
 
  // Pinboard erstellen
  Yii::app()->Pin->CreatePin($data);
 }

 $model = TblProjekt::model()->FindByPk(29); ?>

0

#2 User is offline   SleepWalker 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 16
  • Joined: 02-June 12
  • Location:Ukraine

Posted 09 May 2013 - 11:48 AM

Hallo beat,

diese Zeile brauchst du nicht:
$data = new TblProjekt;



du kannst auch anstatt
...
}
else
{
  if(...) 
  {
  ...
  }
  else
  {
  ...
  }
}

so schreiben:
...
}
elseif(...) 
{
  ...
}
else
{
  ...
}



vielleicht diese Zeile ist sinnlos?
$form->idfs_Faehigkeit = $form->idfs_Faehigkeit;

genauso wie diese:
$model = TblProjekt::model()->FindByPk(29);



Mailversand würde ich als ein Method von `TblProjekt` Model implementieren.

Du könntest auch weniger Verzweigungen machen mit Benutzung von Yii::app()->end() nach der `render()` Function.

Falls du willst, du könntest Anhand der Rules den Gastzugriff begrenzen (Ich weiß aber nicht, ob du deine eigene Nachricht einstellen kannst).

Meiner Meinung nach, $var1 ist unverständliche Name für Variable.

Nachrichten wie 'alert' könnte man mit Yii::app()->user->setFlash() in gesamte Seite Layout integrieren. Wenn du würdest so machen, dann wird es vielleicht für dich ein `render()` Funktion pro `actionProject` Method genug!

P.S. alle diese Tipps es ist nur wie es implementieren könnte. Das bedeutet nicht, dass jeder so tun muss. Jeder hat sein eigenen Codestil. Es kann aber sein, dass in mein Post etwas eintreten wird, dass du nicht über Yii weißt.

BG
Sviat
houath - simple integration with social network authorization on Yii
1

#3 User is offline   Mike 

  • Elite Member
  • PipPipPipPipPip
  • Yii
  • Group: Members
  • Posts: 3,013
  • Joined: 06-October 08
  • Location:Upper Palatinate

Posted 10 May 2013 - 02:42 AM

Da du explizit danach fragst: Ich hab bereits nach wenigen Zeilen aufgehört, deinen Code zu lesen, weil die Einrückungen (Indentation) nicht passen. Das erschwert das Lesen für andere ungemein. Daher als Tipp: Gewöhn dir das an. Sauber eingerückter Code macht es nicht nur dir, sondern auch anderen leichter.

Siehe auch hier: http://de.wikipedia....C3%BCckungsstil



1

#4 User is offline   mbi 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 604
  • Joined: 08-May 09

Posted 10 May 2013 - 06:43 AM

weitere Stichworte: Coding Style, Coding Standard, Konventionen
zum Beispiel TblProjekt vs Tbl_Nachrichten (einerseite Singular/Plural, andererseits PrefixEntity/PrefixUnderscoreEntity)
codesprache englisch vs landessprache (Nachricht vs User, Yii::app()->mail->Send_Bewerbung())

...
1

#5 User is offline   beat78 

  • Junior Member
  • Pip
  • Yii
  • Group: Members
  • Posts: 53
  • Joined: 13-May 11
  • Location:Zürich

Posted 12 May 2013 - 03:51 PM

Hallo Zusammen

Vielen Dank für die wertvollen Tipp's. Habe die Konventionen überarbeitet und die Tipp's umgesetzt.
Das ich mit meiner doch eher geringen Erfahrung noch keinen Preis gewinne bin ich mir bewusst. Darf ich nochmals Fragen ob der Code so im Durchschnitt der Lesbarkeit und Wartbarkeit liegt oder liegen noch Grundlegende Fehler vor?

    public function actionCreateProjektTeilnehmen() {

        $modelStatus = new Tbl_Status;
        $modelNachricht = new Tbl_Nachricht;

        // Prüfen ob Gast und filtern 
        if (isset($_GET['id']) && !Yii::app()->user->isGuest) {

            $data = Tbl_Projekt::model()->findAllByPk($_GET['id']);
        } else {
            $data = Tbl_Projekt::model()->findAll();
        }

        // Prüfen ob Member
        if (Yii::app()->user->isGuest) {

            $msg = "Bitte zuerst anmelden.";  // 
            Yii::app()->user->setFlash('leer', $msg);
            $this->render('Pinnwand', array('data' => $data));
        } else {

            if (isset($_POST['Tbl_Status'])) {

                // Nachricht eintragen
                $modelNachricht->setAttributes($_POST['Tbl_Nachricht']);
                $modelNachricht->idfs_User = Yii::app()->user->id;
                $modelNachricht->idfs_Projekt = $_GET['id'];


                // Status eintragen
                $modelStatus->setAttributes($_POST['Tbl_Status']);
                $modelStatus->idfs_Bewerber = Yii::app()->user->id;
                $modelStatus->idfs_Projekt = $_GET['id'];
                $modelStatus->Datum = time();
                $modelStatus->Status = "Anfrage offen";

                // Formular Verarbeitung
                if ($modelStatus->validate() && $modelNachricht->validate()) {

                    $modelNachricht->save();
                    $modelStatus->Save();

                    // Mail versenden
                    $Projekt = Tbl_Projekt::model()->findByPK($_GET['id']);
                    $User = User::model()->findByPK($Projekt->Benutzer);
                    $AnbieterEmail = $User->username;
                    $this->Send_Bewerbung($AnbieterEmail, $Projekt);

                    // Information
                    $msg = "Die Bewerbung wurde erfolgreich an den Projektleiter versendet.";
                    Yii::app()->user->setFlash('leer', $msg);
                    $data = Tbl_Projekt::model()->findAll();
                    $this->render('Pinnwand', array('data' => $data, 'modelStatus' => $modelStatus, 'modelNachricht' => $modelNachricht));
                }
            }

            // Formular ist noch leer

            $msg = "Bitte wählen Sie die Fähigkeit aus";
            Yii::app()->user->setFlash('leer', $msg);

            $this->render('Pinnwand', array('data' => $data, 'modelStatus' => $modelStatus, 'modelNachricht' => $modelNachricht));
        }
    }




<?php

// Pinwand Zeigen
Yii::app()->Pin->PinnwandZeigen($data);

if (isset($modelStatus)) {

    // Fähigkeiten auswählen
    $this->renderPartial('myform', array('modelStatus' => $modelStatus, 'modelNachricht' => $modelNachricht));
}
?>



<div class="form">

    <?php
    $form = $this->beginWidget('CActiveForm', array(
        'id' => 'tbl-status-status-form',
        'enableClientValidation' => true,
        'enableAjaxValidation' => false,
        'clientOptions' => array(
            'validateOnSubmit' => false,
        ),
        'htmlOptions' => array(
            'enctype' => 'multipart/form-data',
        ),
    ));
    ?>

    <?php //echo $form->errorSummary($modelStatus); ?>

    <div class="row">
        <?php echo $form->labelEx($modelStatus, 'F&auml;higkeit ausw&auml;hlen'); ?>
        <?php echo $form->dropDownList($modelStatus, 'idfs_Faehigkeit', CHtml::listData(Tbl_Faehigkeit::model()->findAll(), 'primary', 'Faehigkeit')); ?>
        <?php echo $form->error($modelStatus, 'idfs_Faehigkeit'); ?>

        <?php echo $form->labelEx($modelNachricht, 'Nachricht Initiant'); ?>
        <?php echo $form->textArea($modelNachricht, 'Nachricht', array('size' => 10, 'maxlength' => 50, 'style' => 'height:200px;')); ?>
        <?php echo $form->error($modelNachricht, 'Nachricht'); ?><br>
        <?php echo CHtml::submitButton('Bewerben'); ?>

    </div>

    <?php $this->endWidget(); ?>

</div><!-- form -->


Posted Image
0

#6 User is offline   Mike 

  • Elite Member
  • PipPipPipPipPip
  • Yii
  • Group: Members
  • Posts: 3,013
  • Joined: 06-October 08
  • Location:Upper Palatinate

Posted 13 May 2013 - 01:45 AM

Sieht doch gleich viel lesbarer aus :).

Die Logik sieht auch o.k. aus, soweit ich die gewünschte Funktion erahnen kann. Du hast allerdings zwei mal die Prüfung isGuest drin. Wenn du die Models erst im else-Block von "Prüfen ob Member" weiter unten lädst, kannst du das erste "&& !Yii::app()->user->isGuest" eliminieren.
1

#7 User is offline   beat78 

  • Junior Member
  • Pip
  • Yii
  • Group: Members
  • Posts: 53
  • Joined: 13-May 11
  • Location:Zürich

Posted 13 May 2013 - 12:45 PM

Hallo Mike,
Vielen Dank für das Feedback und den erneuten Tipp.
Gruss Beat
0

Share this topic:


Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users