# Laravel controller for a grocery list

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()); }  • A controller is supposed to control the Model layer, and not be a Model's service itself. – Kid Diamond Aug 12 '15 at 10:11 • Thanks, I still got a lot to learn. I will have to check what I should put in the model then :) – Hardist Aug 12 '15 at 10:14 ## 1 Answer 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);
}

• Thanks, this indeed works better and is what I was looking for. I learned a lot from this answer :) For my "other problem" I already know what to do. I think I will check if any value is 0 AFTER the update and then update all other values to 0 as well in that row. To keep my form clean. Thanks again. – Hardist Aug 12 '15 at 10:11