最佳实践 - Laravel Controller Eloquent 合并

Question

我的供应商模型上有一个范围，它返回active = true 的结果。

这在创建新条目时非常有用，因为我只希望用户看到活跃的供应商。

当前条目可能有一个不活跃的供应商；当我编辑它时，我想查看所有活动的供应商，以及当前的供应商（如果它是非活动的）

我的控制器中有这段代码：

        $suppliers = Supplier::active()->get();
        if (!$suppliers->contains('id', $record->supplier->id))
        {
            $suppliers->add(Supplier::find($record->supplier->id));
        }

两个问题：这是正确的方法吗？这段代码应该在我的控制器中还是应该在其他地方？ （也许是一个范围，但我不知道如何编码）。

---

编辑：

感谢各位的帮助。我已经从每个答案中应用了建议，并将我的代码重构到一个新的范围内：

    public function scopeActiveIncluding($query, Model $model = null)
    {
        $query->where('active', 1);
        if ($model && !$model->supplier->active)
        {
            $query->orWhere('id', $model->supplier->id);
         }
    }

Answer 1

我看到@Vince 对第一个问题的回答，我同意他的看法。 关于第二个问题：

在供应商模型中这样写范围：

public function scopeActive($query){
    $query->where('active', 1); // for boolean type
}

为了获得良好的实践，您需要在“App\Services\SupplierService.php”等服务中编写逻辑部分。并在那里编写你想要的功能：

public function activeSuppliersWithCurrent($record) {
    $suppliers = Supplier::active()->get();
    $supplier = Supplier::find($record->supplier->id);
    if (!$supplier->active) {
        $suppliers->add($supplier);
    }
}

在您的 SupplierController 的构造函数中注入该服务的实例并使用该函数，例如：

use App\Servives\SupplierService;

protected $supplierService = null;

public function __construct(SupplierService $supplierService) {
    $this->supplierService = $supplierService;
}

public function getActiveSuppliersWithCurrent(...) {
   $result = $this->supplierService->activeSuppliersWithCurrent($record);
}

如您所见，稍后您将不需要更改控制器中的任何内容。如果您需要更改例如供应商选择查询，您只需更改服务中的某些内容。这种方式将使您的代码块分开且更短。 还有这种模式的意义：您不需要从控制器访问模型。所有与模型相关的逻辑都将在服务中实现。 对于其他项目，您可以仅获取服务或仅获取控制器，并以不同的方式实现另一部分。但是在这种情况下，如果您在控制器中拥有所有代码，那将阻止您获取必要代码的部分，因为您可能不记得每个块做了什么......

Answer 2

您可以在查询中添加一个 where 子句来查找该 ID。

$suppliers = Supplier::active()->orWhere('id', $record->supplier->id)->get();

您可以通过传递 'id' 作为参数将其滑入 active 范围。

public function scopeActive($query, $id = null)
{
    $query->where('active', true);

    if ($id) {
        $query->orWhere('id', $id);
    }
}

Supplier::active($record->supplier->id)->get();

或者制作另一个这样做的范围。

Answer 3

您编写的内容会起作用，但如果集合很大，Collection::contains 函数可能会很慢。

既然你有 id，我可能会做出以下改变：

$suppliers = Supplier::active()->get();
$supplier = Supplier::find($record->supplier->id);
if (!$supplier->active) {
  $suppliers->add($supplier);
}

当然，这样做的缺点是您可能对数据库进行了不必要的查询。

所以你必须考虑：

• record 的供应商更有可能是活跃的还是不活跃的？
• 活动供应商集合的规模是否足以证明对数据库进行另一次（可能浪费的）调用是合理的？

根据您对应用程序数据的了解，做出最有意义的选择。

---

至于第二个问题，如果您的应用程序的这一部分只需要这组特定的供应商，那么控制器是这段代码的好地方。

但是，如果您在应用程序的其他部分需要这组特定的供应商，您可能应该将此代码移到其他地方。在这种情况下，在相关模型（无论$record 是什么类型...）上创建一个返回该模型的供应商集的函数可能是有意义的。比如：

public function getSuppliers()
{
  $suppliers = Supplier::active()->get();
  $supplier = $this->supplier;

  if (!$supplier->active) {
    $suppliers->add($supplier);
  }

  return $suppliers;
}