SOLID - gilt das Single-Responsibility-Prinzip für Methoden in einer Klasse?

Question

Ich bin nicht sicher, ob diese Methode in meiner Klasse Einzel Verantwortung Prinzip verstößt,

public function save(Note $note) 
{ 
    if (!_id($note->getid())) { 

     $note->setid(idGenerate('note')); 

     $q = $this->db->insert($this->table) 
         ->field('id', $note->getid(), 'id'); 

    } else { 
     $q = $this->db->update($this->table) 
         ->where('AND', 'id', '=', $note->getid(), 'id'); 
    } 

    $q->field('title', $note->getTitle()) 
     ->field('content', $note->getContent()); 

    $this->db->execute($q); 

    return $note; 
} 

Grundsätzlich macht es zwei Jobs in einem Verfahren - einfügen oder aktualisieren.

Sollte ich getrennt es in zwei Methoden statt, um Single-Verantwortung-Prinzip zu erfüllen?

Aber SRP ist für Klassen nur gemeint, nicht wahr? Gilt es für die Methoden innerhalb einer Klasse?

SRP -

eine Klasse sollte nur eine einzige Verantwortung haben (dh nur ein Potentialänderung in der Spezifikation des Software sollte in der Lage sein, um die Spezifikation der Klasse beeinflussen)

EDIT:

Eine andere Methode zum Auflisten von Notizen (einschließlich vieler verschiedener Art von Auflistungen), Suche n otes, etc ...

public function getBy(array $params = array()) 
{ 
    $q = $this->db->select($this->table . ' n') 
        ->field('title') 
        ->field('content') 
        ->field('creator', 'creator', 'id') 
        ->field('created_on') 
        ->field('updated_on'); 

    if (isset($params['id'])) { 
     if (!is_array($params['id'])) { 
      $params['id'] = array($params['id']); 
     } 

     $q->where('AND', 'id', 'IN', $params['id'], 'id'); 
    } 

    if (isset($params['user_id'])) { 
     if (!is_array($params['user_id'])) { 
      $params['user_id'] = array($params['user_id']); 
     } 

     # Handling of type of list: created/received 
     if (isset($params['type']) && $params['type'] == 'received') { 
      $q 
       ->join(
        'inner', 
        $this->table_share_link . ' s', 
        's.target_id = n.id AND s.target_type = \'note\'' 
       ) 
       ->join(
        'inner', 
        $this->table_share_link_permission . ' p', 
        'p.share_id = s.share_id' 
       ) 
       # Is it useful to know the permission assigned? 
       ->field('p.permission') 
       # We don't want get back own created note 
       ->where('AND', 'n.creator', 'NOT IN', $params['user_id'], 'uuid'); 
      ; 

      $identity_id = $params['user_id']; 

      # Handling of group sharing 
      if (isset($params['user_group_id']) /*&& count($params['user_group_id'])*/) { 
       if (!is_array($params['user_group_id'])) { 
        $params['user_group_id'] = array($params['user_group_uuid']); 
       } 

       $identity_id = array_merge($identity_id, $params['user_group_id']); 
      } 

      $q->where('AND', 'p.identity_id', 'IN', $identity_id, 'id'); 

     } else { 
      $q->where('AND', 'n.creator', 'IN', $params['user_id'], 'id'); 
     } 
    } 

    # If string search by title 
    if (isset($params['find']) && $params['find']) { 
     $q->where('AND', 'n.title', 'LIKE', '%' . $params['find'] . '%'); 
    } 

    # Handling of sorting 
    if (isset($params['order'])) { 
     if ($params['order'] == 'title') { 
      $orderStr = 'n.title'; 

     } else { 
      $orderStr = 'n.updated_on'; 
     } 

     if ($params['order'] == 'title') { 
      $orderStr = 'n.title'; 

     } else { 
      $orderStr = 'n.updated_on'; 
     } 

     $q->orderBy($orderStr); 

    } else { 
     // Default sorting 
     $q->orderBy('n.updated_on DESC'); 
    } 

    if (isset($params['limit'])) { 
     $q->limit($params['limit'], isset($params['offset']) ? $params['offset'] : 0); 
    } 

    $res = $this->db->execute($q); 

    $notes = array(); 

    while ($row = $res->fetchRow()) { 
     $notes[$row->uuid] = $this->fromRow($row); 
    } 

    return $notes; 
} 

Answer 1

Verfahren bleibt die Notiz in die Datenbank. Wenn es das ist, dann ist das eine einzige Verantwortung und die Umsetzung ist in Ordnung. Sie müssen die Logik der Entscheidung, ob irgendwo einfügen oder zu aktualisieren, dies scheint so gut ein Ort wie jeder.

Nur wenn Sie jemals explizit Einfügungen oder Aktualisierungen ohne die implizite Entscheidungslogik durchführen mussten, würde es sich lohnen, diese beiden in verschiedene Methoden zu trennen, die getrennt aufgerufen werden können. Aber im Moment vereinfacht das Beibehalten derselben Methode den Code (da die zweite Hälfte geteilt wird), was wahrscheinlich die beste Implementierung ist.

exempli gratia:

public function save(Note $note) { 
    if (..) { 
     $this->insert($note); 
    } else { 
     $this->update($note); 
    } 
} 

public function insert(Note $note) { 
    .. 
} 

public function update(Note $note) { 
    .. 
} 

Die oben würde Sinn machen, wenn Sie manchmal insert oder update ausdrücklich aus irgendeinem Grund zu nennen brauchte. SRP ist jedoch nicht wirklich ein Grund für diese Trennung.

Answer 2

SOLID-Prinzipien werden auf Terminologie auf Klassenebene angewendet, sie geben keine expliziten Angaben über Methoden. Ein SRP selbst gibt an, dass Klassen einen Grund haben sollten, sich zu ändern, so lange du eine Verantwortung ersetzen kannst, die in einer Klasse verpackt ist, bist du okay.

Bedenken Sie:

$userMapper = new Mapper\MySQL(); 
// or 
$userMapper = new Mapper\Mongo(); 
// or 
$userMapper = new Mapper\ArangoDb(); 

$userService = new UserService($userMapper); 

All diese Mapper eine Schnittstelle implementieren und eine Verantwortung dienen - sie Zugang abstrakte Speicher für Benutzer. Daher haben Mapper einen Grund, sich zu ändern, da Sie sie leicht austauschen können.

Ihr Fall ist nicht über die SRP im Allgemeinen. Es geht mehr um Best-Practice.Nun, die beste Methode in Bezug auf Methoden besagt, dass sie nur eine Sache tun sollten, wann immer es möglich ist und so wenig Argumente wie möglich akzeptieren. Das erleichtert das Lesen und Finden von Fehlern.

Es gibt ein weiteres Prinzip, das Principle of Least Astonishment heißt. Es besagt lediglich, dass Methodennamen explizit das tun sollten, was ihre Namen bedeuten.

kommend bis zu Ihrem Codebeispiel:

Die save() bedeutet, dass es geht um Daten zu speichern (Aktualisierung bestehender Datensatz), nicht zu schaffen. Indem Sie dort einfügen und aktualisieren, brechen Sie die PoLA.

Das ist es, wenn Sie explizit insert() aufrufen Sie wissen und erwarten, dass es einen neuen Datensatz hinzufügen wird. Das gleiche über update() Methode - Sie wissen und erwarten, dass es eine Methode aktualisiert, wird es keine neue erstellen.

Daher werde ich nicht beide Dinge in save() tun. Wenn ich einen Datensatz aktualisieren möchte, würde ich update() anrufen. Wenn ich einen Datensatz erstellen möchte, wähle ich insert().