Please check out my code below. I know I might be asking a lot, but this is newbie code which I feel can be improved quite a lot. So far I haven't been able to get it "better", or "simpler".
What does the code do?
The code first grabs all goals from the database. A goal is a simple table:
CREATE TABLE `goals` (
`id` int(10) NOT NULL,
`name` varchar(255) NOT NULL,
`ratio` int(10) NOT NULL,
`command` varchar(255) NOT NULL,
`created_at` timestamp NULL DEFAULT NULL,
`updated_at` timestamp NULL DEFAULT NULL
)
INSERT INTO `goals` (`id`, `name`, `ratio`, `command`, `created_at`,
`updated_at`) VALUES
(1, 'Test Goal', 50, 'daily', '2017-01-04 08:09:12', '0000-00-00 00:00:00');
It also grabs all logs from today, again, a simple table:
CREATE TABLE IF NOT EXISTS `logs` (
`id` int(10) unsigned NOT NULL,
`user_id` int(10) unsigned NOT NULL,
`action_id` int(10) unsigned NOT NULL,
`date` datetime NOT NULL,
`created_at` timestamp NULL DEFAULT NULL,
`updated_at` timestamp NULL DEFAULT NULL
)
INSERT INTO `logs` (`id`, `user_id`, `action_id`, `date`, `created_at`,
`updated_at`) VALUES
(1, 1, 1, '2017-01-02-11-22-58', '2017-01-02 06:30:43', '2017-01-02
10:22:58', '2017-01-02 10:22:58'),
(2, 1, 2, '2017-01-02-11-23-14', '2017-01-02 07:57:59', '2017-01-02
10:23:14', '2017-01-02 10:23:14'),
(3, 1, 4, '2017-01-02-11-23-23', '2017-01-02 08:12:09', '2017-01-02
10:23:23', '2017-01-02 10:23:23');
Afterwards, I grab all related action_id's
which are related to each goal, and put them into an array. I also grab all action_id's
from all logs and put them into an array as well.
Next, I check how many action_id's
are the same in both arrays. Then, I calculate the percentage based on those results, and perform an if check, and if it passes, I put a certain value into the database.
<?php
use App\Models\Log;
use App\Models\Goal;
use Carbon\Carbon;
Route::get('test', function() {
// Grab all goals where the command is set to "daily"
$goals = Goal::where('command', 'daily')->get();
// Grab all logs from today
$logs = auth()->user()->logs()->whereDate('date', '=', Carbon::today()->toDateString())->paginate(25);
$action_array = [];
// Loop over all goals
foreach($goals as $goal)
{
// Loop over all actions related to the given goal and put the action_id in the $action_array
foreach($goal->actions as $action)
{
$action_array[] = $action->id;
}
}
$log_array = [];
// Loop over all logs and put the action_id related to each log in the $log_array
foreach($logs as $log)
{
$log_array[] = $log->action->id;
}
// Count the total number of actions from each log from today that are equal to the goal actions
$log_count = count(array_intersect($action_array, $log_array));
// Loop over each goal again
foreach($goals as $goal)
{
// Count the total number of actions related to each goal
$action_count = count($goal->actions);
// Calculate the percentage of
$total = $action_count / $log_count * 100;
// Check wether the total percentage of completed logs is equal to or bigger than the goal percentage
if($total >= $goal->ratio)
{
// Insert a value into the database, not important for this example
}
}
});
1 Answer 1
I think your biggest takeaway from this exercise is that, while using "natural language" query builders like those frequently included in popular frameworks represent an easy way for new developers to work with a database, they can also lead to really bad implementation decisions, especially in cases like this where you are trying to query information across a number of tables, or perform aggregate calculations that are best done in the database.
I think you should be looking to run a query like what I have shown below to get information on the percentage completion of actions for each goal. This is assuming there is an actions
table related to goals
as noted in your comments.
SELECT
goals.id AS goal_id,
goals.name AS goal_name,
COUNT(actions.id) AS action_count,
COUNT(logs.actions_id) AS completed_action_count,
((completed_action_count / action_count) * 100) AS percentage_complete
FROM goals
INNER JOIN actions
ON goals.id = actions.goal_id
LEFT JOIN logs
ON actions.id = logs.action_id
GROUP BY
goal_id
ORDER BY
goal_id ASC /* you could obviously order by whichever field you are interested in */
This will save you a lot of juggling of information in your PHP application, as you are now fully leveraging the relational and aggregation capabilities of the database.
If you ever find yourself trying to pull contents from two related tables into your application in order to compare them, this should be a red flag to you that you might need to look at using your framework's join or raw SQL query capability (they pretty much all have this), in order to execute your query.
You could potentially even go a step further and go directly to updating your database as well based on the results for this query. You could add a WHERE percentage_complete >= ?
clause here to further limit the result set to those where you completion ratio is what you desire and perhaps use INSERT ... SELECT
syntax to directly insert information from this query into an appropriate table.
Don't be afraid to venture forth into better understanding the full capabilities of SQL. There are a number of on-line resources like StackOverflow that can help you out if you are having problems getting a query to work.
goals
table does not have a field calledactions
so it is unclear to me where$goal->actions
is populated from or really just how goals are related to actions. Also, why paginate logs query if you are trying to compare all actions against all logs? My guess is that you can get the values you are looking for directly with a single query, but I can;t really give suggestion without understanding these two points. \$\endgroup\$