Will you look at this code and tell me if it can be improved in any way?

[mslilafowler]

A few days back I asked for help on this website, and after some answers pointed me in the right direction I’ve scratched my last approach and taken a new one on how I create this program.

The new solution works exactly the way I want it to, however, I am not too confident that it’s written the best way. I’m a beginner, so the code took many attempts before it worked.

Basically all I’m trying to do is display an HTML table with a list of dates and date-differences from a database, the values coming from input fields on the webpage.

If possible, would anyone be able to tell me if my code is semantically correct?

What it does:

1. Gets last 3 rows from database
2. Fetches user input from a web form
3. Inserts (some) user input into database
4. Takes (remaining) user input to calculate a sum
5. Updates database row and sets the empty/null columns

function secondsToTime($seconds) {
    $dtF = new DateTime("@0");
    $dtT = new DateTime();
    $dtT->setTimestamp($seconds);
    return $dtF->diff($dtT)->format('%a days, %h hours and %i minutes');
}

function secondsToDay($days) {
    $dtF = new DateTime('@0');
    $dtT = new DateTime();
    $dtT->setTimestamp($days);
    return $dtF->diff($dtT)->format('%a');
}

if (isset($_GET['submitPeriod'])) {

	$resultAvg = mysqli_query($conn, "SELECT sum(elapsed) FROM (SELECT elapsed FROM periods ORDER BY id DESC LIMIT 0, 3) AS elapsedNew"); //select last 3 rows
	$estimateNew = mysqli_fetch_assoc($resultAvg)['sum(elapsed)']/2;
	$d_o = date_format(date_create($_GET['d_o']), "Y-m-d H:i:s");
	$d_i = date_format(date_create($_GET['d_i']), "Y-m-d H:i:s");
	$n_e = secondsToDay($estimateNew);
	$notes = $_GET['notes'];

	//Calculate duration of period
	$datetime1 = new DateTime($d_o);
	$datetime2 = new DateTime($d_i);
	$interval = $datetime1->diff($datetime2);
	$elapsed = $interval->format('%a days %h hours %i minutes');

	//Calculate next estimated day depending on value provided
	$estimatedNumber = $n_e;
	$estimatedDate = new DateTime($d_i);
	$estimatedDate->modify('+'.$estimatedNumber.' days');
	$estimate = $estimatedDate->format('Y-m-d H:i:s');

	mysqli_query($conn, "
		INSERT INTO periods 
		VALUES (null, null, '$d_o', '$d_i', '$elapsed', '$estimate', '$n_e days', '$notes')
		");

	//Calculate duration since last period
	$getElapsed = mysqli_query($conn, "
		SELECT TIMESTAMPDIFF(second, (SELECT d_i FROM periods ORDER BY ID DESC LIMIT 1, 1), (SELECT d_o FROM periods ORDER BY ID DESC LIMIT 1)) AS elapsedTime 
		FROM periods LIMIT 1");
	$elapsedResult = mysqli_fetch_assoc($getElapsed)['elapsedTime'];

	mysqli_query($conn, "
		UPDATE periods 
		SET elapsed='$elapsedResult' 
		WHERE id=(SELECT id ORDER BY ID DESC LIMIT 1) 
		ORDER BY ID DESC LIMIT 1 
		 ");
	
	header('Location: ?');
}

I am using 3 stages (insert, select, update) to complete the program before the page is updated.

What I’d like to know is if the code is written badly, and what should be done to improve it. Such advice would be invaluable to me.

Also, if this is not the place to discuss it, please advise where I could receive input.

[astonecipher]

Inherently, I don’t see anything wrong. I mean, you are using variables in the sql string which is bad, but those values don’t appear to be coming from any type of user input. So, it isn’t as bad as it could be.

The only thing I can really say is difficult to explain without some type of test data to actually visualize what I’m talking about. And that is, SQL is non-sequential, meaning, you can do what you are doing internally all at once. I think this will show more what I mean,

SELECT 
	TIMESTAMPDIFF(second, d_i, d_o) AS elapsedTime 
FROM 
	periods

Or if you need to limit the result set,

SELECT 
	TIMESTAMPDIFF(second, d_i, d_o) AS elapsedTime 
FROM 
	periods
ORDER BY 
        id DESC 
LIMIT 3

You can also do updates across the board like this.

UPDATE periods 
        SET elapsed= TIMESTAMPDIFF(second, d_i, d_o)
WHERE 
        id=(SELECT MAX(id) FROM periods)

So, I guess what I am getting at, while there is nothing really wrong with how you are doing it other than from a performance standpoint, you can do what you are doing easier as well.

[mslilafowler]

So I could improve it from a performance standpoint plus find an easier way to code it… Got that.

I intend to apply the use of prepared statements afterward… I find it easier to first create a working program and then apply the security last.

Thanks a lot, mate. Your answer proved helpful once again