Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am curious to know if there is a faster, better, and more efficient way to accomplish this program that acquires employee information:

    <?php
session_start();
//require 'functions.php';
//require 'DB.php';
$employeeID = $_SESSION['employeeID'];
$clockIn = $_GET['clockIn'];
$clockOut = $_GET['clockOut'];
$timeToday = date("g:i a");
$dateToday = date("m/d/y");
$jobDescription = $_GET['jobDesc'];
$equipType = $_GET['equipTypeRan'];
$unitNumber = $_GET['unitNumber'];
$unitHours = $_GET['unitHours'];
if (isset($clockIn)) {

    echo "You clocked in at: " . $timeToday . " on " . $dateToday;

    try {
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare('INSERT INTO timeRecords (employeeID, date, timeIn) VALUES (:employeeID, :dateToday, :timeIn)');

    $stmt->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday, ':timeIn' => $timeToday));

    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }
    $conn = null;


} else if (isset($clockOut)) {

        try {
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare('UPDATE `timeRecords` SET `timeOut`= :timeOut WHERE `date`= :dateToday AND `employeeID`= :employeeID');

    $stmt->execute(array(':employeeID' => $_SESSION['employeeID'],':dateToday' => $dateToday, ':timeOut' => $timeToday));

    $stmt = $conn->prepare('SELECT `timeIn` FROM `timeRecords` WHERE `date`= :dateToday AND `employeeID` = :employeeID');
    $stmt->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday));
    $stmt->setFetchMode(PDO::FETCH_BOTH);
    $results = $stmt->fetch();
    $timeInDB = $results[0];

    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

    $time_one = new DateTime($timeInDB);
    $time_two = new DateTime($timeToday);
    $difference = $time_one->diff($time_two);

    echo "You clocked in at: " . $timeInDB . "<br>";
    echo "You clocked out at: " . $timeToday . "<br>";

    echo $difference->format('Total working time %h hours %i minutes');

}else {

try{
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare("SELECT COUNT(*) FROM timeRecords WHERE `timeOut` IS NULL AND `employeeID`= :employeeID AND `date`= :dateToday");

    $stmt->execute(array(':employeeID' => $employeeID, ':dateToday' => $dateToday));
    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

if($stmt->fetchColumn() > 0){

    try{
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare('UPDATE `timeRecords` SET `jobDescription`= :jobDescription, `equipType`= :equipType, `unitNumber`= :unitNumber, `unitHours`= :unitHours, 
                            `timeOut`= :timeOut WHERE `employeeID`= :employeeID AND `date`= :dateToday AND `timeOut` IS NULL');

    $stmt->execute(array(':employeeID' => $employeeID, ':timeOut' => $timeToday, ':dateToday' => $dateToday, ':jobDescription' => $jobDescription,
                         ':equipType' => $equipType, ':unitNumber' => $unitNumber, ':unitHours' => $unitHours));

    $stmt = $conn->prepare('INSERT INTO timeRecords (employeeID, date, timeIn) VALUES (:employeeID, :dateToday, :timeIn)');
    $stmt ->execute(array(':employeeID' => $employeeID, ':dateToday'=> $dateToday, ':timeIn' => $timeToday));
    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

    echo "There was an update";
} else {
    echo "There was no Match";
}

    require 'summary.php';

}

?>
share|improve this question
4  
Why make the db connection so many times? Why not only once? –  Rob Apodaca Jan 13 '13 at 17:56
2  
I guess there is not much to improve performance-wise (not in a noticeable order). However there's a lot of code duplication in there which could be reduced. –  Fge Jan 16 '13 at 7:59
    
setting PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION will do exactly the same as wrapping your code in try catch. you can verify by removing them all together, a wrong query will still echo the same messages you echo in your catch part, but without the need of writing all those extra lines. –  Félix Gagnon-Grenier Feb 19 at 14:53

1 Answer 1

Defining the same connection multiple times can be avoided, and also, putting everything in try/catch serves no purpose if the only thing you do in the catch is displaying the message which would be displayed anyway with the error reporting mode you set on the $conn

I also find that naming PDOStatements can help a bit for readability, instead of always using $stmt. Here it all happen in few lines, but sometimes, using a prepared statement far away in the code can be easier if they have significant names.

doing this, I noticed your clockIn PDOStatement id done two times. since the if prevents it from happening two times, it's ok, but maybe you could find some use in preparing your PDOStatements outside of loops, then executing them in the loops.

<?php

session_start();
//require 'functions.php';
//require 'DB.php';
$employeeID = $_SESSION['employeeID'];
$clockIn = $_GET['clockIn'];
$clockOut = $_GET['clockOut'];
$timeToday = date("g:i a");
$dateToday = date("m/d/y");
$jobDescription = $_GET['jobDesc'];
$equipType = $_GET['equipTypeRan'];
$unitNumber = $_GET['unitNumber'];
$unitHours = $_GET['unitHours'];

$conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$putClockIn = $conn->prepare('INSERT INTO timeRecords (employeeID, date, timeIn) VALUES (:employeeID, :dateToday, :timeIn)');

if (isset($clockIn))
 {echo "You clocked in at: " . $timeToday . " on " . $dateToday;
  $putClockIn->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday, ':timeIn' => $timeToday));
  $conn = null;}
else if (isset($clockOut))
 {$putClockOut = $conn->prepare('UPDATE `timeRecords` SET `timeOut`= :timeOut WHERE `date`= :dateToday AND `employeeID`= :employeeID');
  $putClockOut->execute(array(':employeeID' => $_SESSION['employeeID'],':dateToday' => $dateToday, ':timeOut' => $timeToday));
  $getTimeIn = $conn->prepare('SELECT `timeIn` FROM `timeRecords` WHERE `date`= :dateToday AND `employeeID` = :employeeID');
  $getTimeIn->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday));
  //$getTimeIn->setFetchMode(PDO::FETCH_BOTH);
  $results = $getTimeIn->fetch(PDO::FETCH_BOTH); // no need to set a mode for stmt if you ain't using it anywhere else, pass it directly in fetch()
  $timeInDB = $results[0];
  $time_one = new DateTime($timeInDB);
  $time_two = new DateTime($timeToday);
  $difference = $time_one->diff($time_two);
  echo "You clocked in at: " . $timeInDB . "<br>";
  echo "You clocked out at: " . $timeToday . "<br>";
  echo $difference->format('Total working time %h hours %i minutes');}
else
 {$getCurrentSessions = $conn->prepare("SELECT COUNT(*) FROM timeRecords WHERE `timeOut` IS NULL AND `employeeID`= :employeeID AND `date`= :dateToday");
  $getCurrentSessions->execute(array(':employeeID' => $employeeID, ':dateToday' => $dateToday));
  if($getCurrentSessions->fetchColumn() > 0)
   {$closeSession = $conn->prepare('UPDATE `timeRecords` SET `jobDescription`= :jobDescription, `equipType`= :equipType, `unitNumber`= :unitNumber, `unitHours`= :unitHours, 
                            `timeOut`= :timeOut WHERE `employeeID`= :employeeID AND `date`= :dateToday AND `timeOut` IS NULL');
    $closeSession->execute(array(':employeeID' => $employeeID, ':timeOut' => $timeToday, ':dateToday' => $dateToday, ':jobDescription' => $jobDescription,
                         ':equipType' => $equipType, ':unitNumber' => $unitNumber, ':unitHours' => $unitHours));
    $putClockIn ->execute(array(':employeeID' => $employeeID, ':dateToday'=> $dateToday, ':timeIn' => $timeToday));
    echo "There was an update";}
  else
   {echo "There was no Match";}}

require 'summary.php';

?>
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.