Laravelコントローラーでのイベントのコードロジックの構造化

Question

以下のコードは、base64画像を別のWebサイトに保存し、Vueページに画像をリアルタイムで表示するイベントを作成します。それはうまく機能します。

私の質問は、これはそれを行うための良い方法ですか、それともコードを構造化するためのより良い方法がありますか？ロジックをリスナーに配置することを考えましたが、使用する必要はないようです。キューについては、後で追加します。

コントローラ

public function store(Request $request) { $base64 = $request->image; $image = str_replace('data:image/png;base64,', '', $base64); $imageName = 'draw'.time().'.png';
    $response = $this->guzzleClient()->post(config('archiving.api_postBase64_file'),
        [  
            'multipart' => [
                [
                    'name' => 'file',
                    'contents' => $image ], [ 'name' => 'file_name', 'contents' => $imageName
                ],
                [
                    'name' => 'file_folder',
                    'contents' => 'drawings'
                ],
            ],
        ]
    );
    if($response->getStatusCode() >= 500){ return response($response->getBody(), $response->getStatusCode()); }else{ $drawing = Drawing::create([
            'user_id'=>Auth::id(),
            'file_name'=>$imageName, ]); event(new NewDrawing($drawing));
        return response('Drawing created!',200);
    }
}

NewDrawingイベント

class NewDrawing implements ShouldBroadcastNow
{
    use Dispatchable, InteractsWithSockets, SerializesModels;

    public $drawing; public function __construct(Drawing $drawing)
    {
        $this->drawing = $drawing;
    }
    public function broadcastOn()
    {
        return new Channel('channel-drawing');
    }
 }

フロントエンド（Vue）

<script>
export default {
    data(){
        return{
            images:'',
        }
    },
    methods:{
        listen() {
            Echo.channel('channel-drawing')
            .listen('NewDrawing', res => {
                this.images.push(res.drawing);
            })  
        }
    },
    mounted() {
        this.listen()
    }
}
</script>

Answer 1

私は主に、フロントエンドコードを大騒ぎするバックエンドの人です。これが私のアドバイスです：

• コントローラコードは、応答のみの処理にできるだけ近づけてください。オブジェクトの作成を独自のクラスに委任します。SOLIDのSについて考えてください-単一責任。

  public function store(Request $request) { $image = new ImageMeta($base64); $archiver = new ImageArchiver($image); $response = $archiver->post(); if($response->getStatusCode() >= 500){
          return response($response->getBody(), $response->getStatusCode());
      }else{
          $drawing = Drawing::create([ 'user_id'=>Auth::id(), 'file_name'=>$imageName,
          ]);
          //There's a few different ways to look at this which might be part of a larger conversation, 
          // so I won't touch this part yet. But is this something specific to drawings? Or is the broadcast
          // of model creation something that will happen for lots of different models? Regardless, I would 
          // change the name from NewDrawing to something like DrawingCreationBroadcast, or something like that
          // NewDrawing gets confusing since its too close to new Drawing();
          event(new NewDrawing($drawing));
          return response('Drawing created!',200);
      }
   }
  

ImageMetaクラス

    /** For all classes assume property declarations, and getters are included. 
    * Lean toward immutable classes where possible, so don't include setters 
    * unless it becomes necessary. Code re-use != object instance re-use.
    **/

    class ImageMeta {
        public function __construct($base64) {
            $this->image = str_replace('data:image/png;base64,', '', $base64);
            $this->imageName = 'draw'.time().'.png';
        }
    }

ImageArchiverクラス

    class ImageArchiver {
        private $config;
        
        public function __construct(ImageMeta $image) { $this->image = $image; $this->makeConfig();
        }
        
        private function makeConfig() {
            $this->config = [ 'multipart' => [ [ 'name' => 'file', 'contents' => $image->getImage()
                    ],
                    [
                        'name' => 'file_name',
                        'contents' => $imageName->getName() ], [ 'name' => 'file_folder', 'contents' => 'drawings' ], ], ] } //use type hinting here. Off the top of my head I don't know the parent class of guzzleClient public function archive($guzzleClient) {
            return $guzzleClient->post($this->config);
        }

    }

Answer 2

では、コードのクリーンアップについては、このプレゼンテーション避けるように-ラファエルDohmsをリーンコードを維持するために多くの方法について語っelseキーワードを。（ここのスライドを参照してください）。

elseキーワードを避けるのが賢明です-特にそれが必要でないとき-例えば前のブロックがreturnステートメントを含んでいるとき：

   if($response->getStatusCode() >= 500){ return response($response->getBody(), $response->getStatusCode()); }else{ $drawing = Drawing::create([
           'user_id'=>Auth::id(),
           'file_name'=>$imageName, ]); event(new NewDrawing($drawing));
       return response('Drawing created!',200);
   }

---

フロントエンドコードでimagesは、空の文字列に設定されているように見えます。

 data(){
   return{
       images:'',
   }
},

しかし、listenコールバックでは、配列のように使用されます。

       Echo.channel('channel-drawing')
       .listen('NewDrawing', res => {
           this.images.push(res.drawing);
       })  

の配列として初期化する必要がありますdata()。

メソッドlisten()はmounted()コールバック以外の場所で呼び出されますか？そうでない場合は、コードをそのメソッドからmounted()コールバックに移動する方が簡単な場合があります。