Problem
What I have is a grocery list. Once I select a grocery by ticking the checkbox, I can select an amount for that product and a unit (unit is referred to in my below code as group). Once I am done, I click save and the values are updated in the database. But for all checkboxes that are unchecked, the values should be updated to 0. Also, if I DO select a checkbox, but I DO NOT select an amount OR a group, the values should all still be updated to 0.
The code below works flawlessly, but I think it could be written much shorter / better. Any pointers?
public function post()
{
$checkboxes = Input::get('checkbox');
$checkedAmounts = Input::get('amount');
$checkedGroups = Input::get('group');
/* The below code checks if a checkbox was checked, an amount was selected and a group was selected. If any of those values are true, update the database with the values selected. */
foreach ($checkedAmounts as $key => $value)
{
if($value !== '0')
{
Product::where('id', '=', $key) ->update(['amount' => $value]);
}
}
foreach ($checkedGroups as $key => $value)
{
if($value !== '0')
{
Product::where('id', '=', $key) ->update(['group' => $value]);
}
}
foreach ($checkboxes as $key => $value)
{
if($value !== '0')
{
Product::where('id', '=', $key) ->update(['selected' => '1']);
}
}
/* The below code checks if a checkbox was NOT checked, an amount was NOT selected or a group was NOT selected. If ANY of those are false, update the database and set everything to 0 for that row. */
foreach ($checkboxes as $key => $value)
{
if($value === '0')
{
Product::where('id', '=', $key) ->update(['selected' => '0']);
Product::where('id', '=', $key) ->update(['amount' => '0']);
Product::where('id', '=', $key) ->update(['group' => '0']);
}
}
foreach ($checkedAmounts as $key => $value)
{
if($value === '0')
{
Product::where('id', '=', $key) ->update(['selected' => '0']);
Product::where('id', '=', $key) ->update(['amount' => '0']);
Product::where('id', '=', $key) ->update(['group' => '0']);
}
}
foreach ($checkedGroups as $key => $value)
{
if($value === '0')
{
Product::where('id', '=', $key) ->update(['selected' => '0']);
Product::where('id', '=', $key) ->update(['amount' => '0']);
Product::where('id', '=', $key) ->update(['group' => '0']);
}
}
return Redirect::to('groceries')->with('categories', Category::with('products')->orderBy('name', 'asc')->get());
}
Solution
First of all, you could have less updates if you passed all values you want to set at once:
Product::where('id', '=', $key) ->update(['selected' => '0', 'amount' => '0', 'group' => '0']);
Secondly, you check in many places in your code if value is different than 0 and save the value, but if later if it is equal to 0 you save 0 in the database – you could just simply save the value in the database and skip the checks.
This, including some other enhancements, should work for you:
$data = [];
foreach ($checkedAmounts as $productId => $value) {
$data[$productId]['amount'] = $value;
}
foreach ($checkedGroups as $productId => $value) {
$data[$productId]['group'] = $value;
}
foreach ($checkboxes as $productId => $value) {
$data[$productId]['selected'] = boolval($value);
}
foreach ($data as $productId => $updates) {
Product::whereId($productId)->update($updates);
}