0
\$\begingroup\$

I feel like this code is pretty sloppy and can be accomplished in a foreach loop. But it's a bit complex using a multi-dimensional array.

Can this be cleaner? You can see I'm repeating much of the same code in each li, minus some class and array key differences, etc. Should this be a foreach loop? What's the best approach?

// data example:
{
 "user": {
 "assigned": 3,
 "overdue": 2,
 "in_review": 1,
 "in_progress": 2
 },
 "team_avg": {
 "overdue": 1,
 "in_review": 1,
 "in_progress": 1,
 "assigned": 1
 }
}
<ul class="large-block-grid-4">
 <li>
 <small>Assigned</small>
 <h4 class="assigned">
 {{ AbbrNum::convert($taskload['user']['assigned']) }}
 @if (count($taskload['team_avg']))
 @if ($taskload['user']['assigned'] > $taskload['team_avg']['assigned'])
 <i data-tooltip aria-haspopup="true" title="Above Team Avg" class="fa fa-arrow-up above-avg" aria-hidden="true"></i>
 @elseif ($taskload['user']['assigned'] == $taskload['team_avg']['assigned'])
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-minus" aria-hidden="true"></i>
 @else
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-arrow-down below-avg" aria-hidden="true"></i>
 @endif
 @endif
 </h4>
 </li>
 <li>
 <small>In Progress</small>
 <h4 class="in-progress">
 {{ AbbrNum::convert($taskload['user']['in_progress']) }}
 @if (count($taskload['team_avg']))
 @if ($taskload['user']['in_progress'] > $taskload['team_avg']['in_progress'])
 <i data-tooltip aria-haspopup="true" title="Above Team Avg" class="fa fa-arrow-up above-avg" aria-hidden="true"></i>
 @elseif ($taskload['user']['in_progress'] == $taskload['team_avg']['in_progress'])
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-minus" aria-hidden="true"></i>
 @else
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-arrow-down below-avg" aria-hidden="true"></i>
 @endif
 @endif
 </h4>
 </li>
 <li>
 <small>In Review</small>
 <h4 class="in-review">
 {{ AbbrNum::convert($taskload['user']['in_review']) }}
 @if (count($taskload['team_avg']))
 @if ($taskload['user']['in_review'] > $taskload['team_avg']['in_review'])
 <i data-tooltip aria-haspopup="true" title="Above Team Avg" class="fa fa-arrow-up above-avg" aria-hidden="true"></i>
 @elseif ($taskload['user']['in_review'] == $taskload['team_avg']['in_review'])
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-minus" aria-hidden="true"></i>
 @else
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-arrow-down below-avg" aria-hidden="true"></i>
 @endif
 @endif
 </h4>
 </li>
 <li>
 <small>Overdue</small>
 <h4 class="overdue">
 {{ AbbrNum::convert($taskload['user']['overdue']) }}
 @if (count($taskload['team_avg']))
 @if ($taskload['user']['overdue'] > $taskload['team_avg']['overdue'])
 <i data-tooltip aria-haspopup="true" title="Above Team Avg" class="fa fa-arrow-up below-avg" aria-hidden="true"></i>
 @elseif ($taskload['user']['overdue'] == $taskload['team_avg']['overdue'])
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-minus" aria-hidden="true"></i>
 @else
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-arrow-down above-avg" aria-hidden="true"></i>
 @endif
 @endif
 </h4>
 </li>
</ul>
asked Apr 23, 2016 at 20:38
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Please edit your title to say something about what the code does rather than what you'd like reviewed. \$\endgroup\$ Commented Apr 23, 2016 at 20:46
  • \$\begingroup\$ The title still doesn't look proper. Please look at the front page for examples on good titles. \$\endgroup\$ Commented Apr 23, 2016 at 22:05

1 Answer 1

1
\$\begingroup\$

I agree that a foreach would be possible, but probably not the best solution here as it would indeed make the code overly complex. There is a lot of repetition though, so there is indeed room for improvement. I think a partial sub view would probably be the cleanest solution here. This is how I would go about it.

in a separate file (partials/list_item.blade.php):

<li>
 <small>{{$title}}</small>
 <h4 class="{{$class}}">
 {{ AbbrNum::convert($taskload['user'][$type]) }}
 @if (count($taskload['team_avg']))
 @if ($taskload['user'][$type] > $taskload['team_avg'][$type])
 <i data-tooltip aria-haspopup="true" title="Above Team Avg" class="fa fa-arrow-up above-avg" aria-hidden="true"></i>
 @elseif ($taskload['user'][$type] == $taskload['team_avg'][$type])
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-minus" aria-hidden="true"></i>
 @else
 <i data-tooltip aria-haspopup="true" title="Below Team Avg" class="fa fa-arrow-down below-avg" aria-hidden="true"></i>
 @endif
 @endif
 </h4>
</li>

And in your existing template:

<ul class="large-block-grid-4">
 @include('partials.list_item', ['title' => 'Assigned', 'type' => 'assigned', 'class' => 'assigned' ])
 @include('partials.list_item', ['title' => 'In progress', 'type' => 'in_progress', 'class' => 'in-progress' ])
 ...
</ul>
answered Apr 23, 2016 at 23:42
\$\endgroup\$
3
  • \$\begingroup\$ Interesting concept! I didn't think to approach it like this - but makes sense. I wonder if there's any performance differences between this and a foreach. That said, if there is, it's likely very low/petty anyways. Thanks for your feedback/help! \$\endgroup\$ Commented Apr 24, 2016 at 0:03
  • \$\begingroup\$ What about foreach'ing the partial? \$\endgroup\$ Commented Apr 24, 2016 at 0:04
  • \$\begingroup\$ Wait, that's essentially the same thing as my original question lol \$\endgroup\$ Commented Apr 24, 2016 at 0:08

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.