最佳实践 - Laravel控制器雄辩合并

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

你写的东西可以工作，但如果集合很大，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;
}

Answer 2

我看到了@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);
}
}

在供应商控制器的构造函数中注入该服务的实例并使用该函数，例如：

use AppServivesSupplierService;
protected $supplierService = null;
public function __construct(SupplierService $supplierService) {
$this->supplierService = $supplierService;
}
public function getActiveSuppliersWithCurrent(...) {
$result = $this->supplierService->activeSuppliersWithCurrent($record);
}

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

Answer 3

您可以向查询添加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();

或者制作另一个范围来执行此操作。