Level 3
The 'logic' would refer more to the modification of data you get from your database or from your user requests.
I wouldn't classify this code as 'logic', it's just there so you present the appropriate select box to the user.
Feb 16, 2017
4
Level 1
Review my logic within view, should I extract?!
As a bit of a newcomer to MVC architecture I understand I should have no logic in my views.
I am testing my skills by writing a basic ecommerce application. I have created a quantity selector in my bag which is displayed for each product. Eventually I will link it up to some AJAX to allow customers to change the quantity within the bag area.
At the moment I have the below logic.
I want the select box to show options up to a quantity of 10 unless the customer has more than 10 items added to their bag in which case it should show up to this amount
@if ($item->qty > 10)
@foreach (range(1, $item->qty) as $quantity)
<option value="{{ $quantity }}" @if ($quantity === $item->qty) selected @endif>{{ $quantity }}</option>
@endforeach
@else
@foreach (range(1, 10) as $quantity)
<option value="{{ $quantity }}" @if ($quantity === $item->qty) selected @endif>{{ $quantity }}</option>
@endforeach
@endif
Should this logic be extracted into something like a helper function? If so, what's the best practice?
Level 1
It just looks a bit messy. Also, this code is also reused on the product page to show the quantity select box so should it be put into a partial?
Level 80
@joe.ainsworth Patterns of repetition are good candidates for re-factoring. Looking at the code above, both the if and the else bodies seem to do the same thing: loop over a range and print an tag for each iteration. The only difference is the upper value. This could be condensed like this:
@foreach (range(1, max($item->qty, 10)) as $quantity)
<option value="{{ $quantity }}" @if ($quantity === $item->qty) selected @endif>{{ $quantity }}</option>
@endforeach
Using the max() function, this will now return the greatest of either 10 or $item->qty. So if $item->qty is less than 10, 10 will be returned. If $item->qty is greater than 10, then the value of $item->qty will be returned.
In HTML tags, I prefer to use ternary operators rather than Blade control structures. It looks cleaner, so your tag would look like this:
<option value="{{ $quantity }}" {{ $quantity == $item->qty ? 'selected' : '' }}>{{ $quantity }}</option>
Putting all this together, you get:
@foreach(range(1, max($item->qty, 10)) as $quantity)
<option value="{{ $quantity }}" {{ $quantity == $item->qty ? 'selected' : '' }}>{{ $quantity }}</option>
@endforeach
Alternatively, you could use the Laravel Collective Forms & HTML package:
Form::select('quantity', range(1, max($item->qty, 10)), $item->qty)
2 likes
Level 47
In a perfect world your view should present data and do no manipulation what so ever.
But the world isn't perfect and sometimes you need to. Sometimes extracting out to a view presenter or a helper just adds to much complexity which leads to excessive amount of work and frustration. By default MVC helps separate things but also connects things that require each other. Catch 22. Experience will dictate when and where to separate things, until then just keep it simple. Learn and move forward. Good question though.
Any case @martinbean has what I would suggest also.
Please or to participate in this conversation.