HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Speeding up class that uses an ODBC connection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
odbcusesthatspeedingclassconnection

Problem

I have created a class that gets data from an ODBC connection. The code works but it is really slow - I'm talking up to 1.20ish minutes to run. I know my code is inefficient but I'm really not sure why it's so slow. Tips or advice on speeding it up would be much appreciated.

```
name = $name;
$this->Id = $Id;
}

Public function Data() {
$conn = odbc_connect('monitor', '', '');
if (!$conn) {
exit("Connection Failed: " . $conn);
}

$sql = "SELECT TOP 1 ReaderData.ReaderIndex, ReaderData.CardID, ReaderData.ReaderDate, ReaderData.ReaderTime, ReaderData.controllerID, Left([dtReading],10) AS [date], ReaderData.dtReading FROM ReaderData
WHERE ReaderData.controllerID=$this->Id AND ReaderData.CardID = 'FFFFFFF0 '
ORDER BY ReaderData.ReaderIndex DESC;";
$rs = odbc_exec($conn, $sql);
if (!$rs) {
exit("Error in SQL");
}
while (odbc_fetch_row($rs)) {

$this->DtReading = odbc_result($rs, "dtReading");

$result = strtotime($this->DtReading) + 2 60 60;

$this->time = time() + 60 * 60 - $result;
return round($this->time / 60, 2);
}
}

public function Cycle() {
$this->Arr = array();
$conn = odbc_connect('monitor', '', '');
if (!$conn) {
exit("Connection Failed: " . $conn);
}

$sql = "SELECT TOP 2 ReaderData.ReaderIndex, ReaderData.CardID, ReaderData.ReaderDate, ReaderData.ReaderTime, ReaderData.controllerID, Left([dtReading],10) AS [date], ReaderData.dtReading FROM ReaderData WHERE ReaderData.controllerID=$this->Id AND ReaderData.CardID = 'FFFFFFF0 ' ORDER BY ReaderData.ReaderIndex DESC;";
$rs = odbc_exec($conn, $sql);
if (!$rs) {
exit("Error in SQL");
}

$data = array();

while (odbc_fetch_row($rs)) {
$data[] = $this->DtReading = odbc_res

Solution

Recycling is the solution

For every Machine, you are performing 3 queries and opening/closing 3 connections.

So for 10 machines, you are performing 30 queries (no problemo) but opening a connection 30 times (w t f?) and closing it again.

There is your problem. Instead of having one connection that you query, you open a connection everytime you need to query something. this is really bad for performance and kills your application.

However, there are multiple things wrong with your code. Not only is it poorly written (it's just a bunch of functions put into a class), the design is simply wrong / there is no design.
Application Design

When writing software, we always solve problems. If the problem can't be solved in a few lines of code, we split the problem into smaller problems that can be solved. If a lot of small problems exist, it is best to create some structure in those solutions. You could for instance bundle them in a class, or namespace or file.
Defining the problem

Getting Data from an ODBC DataSource.

For this, we have a set of functions provided to us by php. the odbc_* functions. Solved. NEXT!

the real problem is that it's a real pain in tha ass writing the exact same odbc_query all over the place. We need an interface to comunicate with that helps us managing MachineObjects that are stored in an odbc MachineDataSource.
Defining the solution

Now we know the problem, we can start splitting it into smaller ones:

  • We need a Machine Objects that holds all the Machine data and adds some extra meaning + calculations for us. (e.g. the GetM() method)



  • I need a MachineDataSource that when I pass in an ID and a Name, it returns me the correct MachineObject



So you need a class that can represent a Machine and you need an interface that creates these Machines for you. I would go for the Repository Pattern here (look it up).
Fine tuning

A good way to fine tune this is to use Eager loading. Instead of performing multiple selects:

SELECT something FROM somewhere WHERE id = 1;
SELECT something FROM somewhere WHERE id = 2;
SELECT something FROM somewhere WHERE id = 3;
SELECT something FROM somewhere WHERE id = 4;
SELECT something FROM somewhere WHERE id = 5;


We could use eager loading and fetch them at the same time:

SELECT something FROM somewhere WHERE id IN(1,2,3,4,5);
//not sure or this query is correct, but you get the point


This however adds a lot of complex stuff to your code, Laravel Eloquent does a good job using a Collection of Objects (Machine's in your story) and then load certain data for all Objects in the Collection.

Code Snippets

SELECT something FROM somewhere WHERE id = 1;
SELECT something FROM somewhere WHERE id = 2;
SELECT something FROM somewhere WHERE id = 3;
SELECT something FROM somewhere WHERE id = 4;
SELECT something FROM somewhere WHERE id = 5;
SELECT something FROM somewhere WHERE id IN(1,2,3,4,5);
//not sure or this query is correct, but you get the point

Context

StackExchange Code Review Q#57381, answer score: 3

Revisions (0)

No revisions yet.