Code review: Optimize <select> form

I have this working code, but I wonder if there is a smarter way to handle it. It all comes down to this form in Blade:

<select name="material_id" class="form-select">
    @foreach ($materials as $material)
        <option value="{{ $material->id }}" {{ ($material->id == $default_material_id) ? 'selected' : '' }}>{{ $material->name }}</option>
    @endforeach
</select>

Then I have this in my controller:

public function index()
{

    # Materials
    $materials = Material::where('active', 1)->get();

    # Set default material
    # Get from database
    $default = Material::where('default', 1)->first();
    $default_material_id = $default->id;

    return view('weight.tube', compact('materials','default_material_id'));

}

public function store(Request $request)
{

    $validated = $request->validate([
        'material_id' => 'exists:materials,id',
        'inside_diameter'=> 'required|integer|max_digits:10',
        'outside_diameter' => 'required|integer|max_digits:10',
        'length' => 'required|integer|max_digits:50',
    ]);

    $inside_diameter = $request->inside_diameter;
    $outside_diameter = $request->outside_diameter;
    $length = $request->length;

    # Find material
    $material = Material::find($request->material_id);

... stuff here

    $data = collect([
        'outside_diameter' => $outside_diameter,
        'inside_diameter' => $inside_diameter,
        'length' => $length,
        'density' => $material->density,
        'cross_section' => $cross_section,
        'grams' => $weight,
    ]);

   # Set default material id
    # Form selected
    $default_material_id = $request->material_id;

    # Get all materials
    $materials = Material::where('active', 1)->get();

    # Flash request data to session
    $request->flash();

    return view('weight.tube', compact('data','materials','default_material_id'));

}

It will set default material in both index() and store() "default_material_id" - so I know what to select in the form. Use default from DB if no form submit, or mark the selected one (if form has been selected.

LaryAI

3 years ago

Level 58

The code looks good, but there is a simpler way to handle the default material. Instead of querying the database twice, you can use the value method to get the default material ID directly. Here's how you can modify the index method:

public function index()
{
    $materials = Material::where('active', 1)->get();
    $default_material_id = Material::where('default', 1)->value('id');

    return view('weight.tube', compact('materials', 'default_material_id'));
}

This will set the $default_material_id variable to the ID of the default material, or null if there is no default material. You can then use this variable in your Blade template to mark the selected option in the dropdown list.

Also, in the store method, you can simplify the code that sets the $data variable using the only method:

$data = collect($request->only(['inside_diameter', 'outside_diameter', 'length']));
$data['density'] = $material->density;
$data['cross_section'] = $cross_section;
$data['grams'] = $weight;

This will create a new collection with only the specified keys from the $request object.

Please or to participate in this conversation.