My thoughts in walking through your code are shown below:
include ('../cfg/connect.php');
This appears to be hard dependency. You should use require
such that this script fails out of you can't load the dependency. Also, your style of adding space between function/method name and opening parenthesis would be considered non-standard by most PHP developers. Typical usage has no space. This makes reader less likely to confuse this parenthesis with a logical operation.
$s = " : ";
$br = "<br>";
$ul = "<ul>";
$li = "<li>";
$_ul = "</ul>";
$_li = "</li>";
Consider using constants for these, as they don't change during script execution. You might also find this helps readability of your code when you begin to concatenate with these.
$categoryQry = "SELECT a.itemQty, b.transDate, b.transID, b.amount, a.itemPrice, a.itemCategory, c.catFamily, a.itemName, a.itemSource FROM budget.lineItems AS a JOIN budget.quickEntry AS b ON a.transID = b.transID JOIN budget.categories AS c ON a.itemCategory = c.catName WHERE b.processed = 'y' ORDER BY c.catFamily, c.catName, b.transDate";
This is a horrendously long line of code. You should strive to keep character count on a line under 80. Break this query up across lines, making it much easier to read.
Example:
$categoryQry = <<<EOT
SELECT
a.itemQty,
b.transDate,
b.transID,
b.amount,
a.itemPrice,
a.itemCategory,
c.catFamily,
a.itemName,
a.itemSource
FROM budget.lineItems AS a
JOIN budget.quickEntry AS b
ON a.transID = b.transID
JOIN budget.categories AS c
ON a.itemCategory = c.catName
WHERE b.processed = 'y'
ORDER BY c.catFamily, c.catName, b.transDate
EOT;
$categories = $conn->prepare ($categoryQry);
$categories->execute ();
$categories->store_result ();
$categories->bind_result ($itemQty, $transDate, $transID, $totalPrice, $itemPrice, $category, $family, $itemName, $source);
This all assumes happy-path execution. What happens if prepare()
, or execute()
fail? Last line of code is too long. Break it up
Example:
$categories->bind_result(
$itemQty, $transDate, $transID, $totalPrice, $itemPrice,
$category, $family, $itemName, $source
);
if (!isset($data[$transID]['amount'])) {
$data[$transID]['amount'] = 0;
}
if (!isset($data[$transID]['line'])) {
$data[$transID]['line'] = '';
}
if (!isset($data[$transID]['line'][$family]['amount'])) {
$data[$transID]['line'][$family]['amount'] = 0;
}
if (!isset($data[$transID]['line'][$family]['line'])) {
$data[$transID]['line'][$family]['line'] = '';
}
if (!isset($data[$transID]['line'][$family]['line'][$category]['amount']))
{
$data[$transID]['line'][$family]['line'][$category]['amount'] = 0;
}
if (!isset($data[$transID]['line'][$family]['line'][$category]['line']))
{
$data[$transID]['line'][$family]['line'][$category]['line'] = '';
}
$itemCount++;
$qtyPrice = $itemPrice * $itemQty;
$data[$transID]['amount'] += $qtyPrice;
$data[$transID]['transDate'] = $transDate;
$data[$transID]['source'] = $source;
$data[$transID]['line'][$family]['amount'] += $qtyPrice;
$data[$transID]['line'][$family]['line'][$category]['amount'] += $qtyPrice;
$data[$transID]['line'][$family]['line'][$category]['line'][$itemName]
= ['itemQty' => $itemQty, 'itemPrice' => $itemPrice];
I have concern over the overall data structure you are building. First, I think there are more levels of nesting than is necessary. Second, the repeated use of line
as key at different levels of depth seems odd and potentially confusing.
I would encourage you to really rethink your data structure with an object-oriented approach. Think about what are the "real world" properties that should exist for each item in the hierarchy and how you represent the relationship between items.
You might end up with a structure like that shown below (shown using JSON-like notation to indicate object structures - or alternately associative arrays in PHP). This has 4 levels of nesting instead of 8 as your current model does. Interestingly enough, you will also note that your logic to output this data structure has 4 levels. That is because four levels of nesting is what the "real-world" relations between these objects in your hierarchy really dictate.
{
'transactionid1': {
'amount': *,
'date': *,
'source': *,
'categories': {
'categorynameA': {
'amount': *,
'lines': {
'linenameA': {
'amount': *,
'items': {
'itemnameA': {
'qty': *,
'price': *
},
'itemnameB': // more items
},
'linenameB': // other lines
},
'categorynameB': // other categories
},
'transactoinid2': // other transactions
}
This isn't exactly optimized for your display loops (which do not require unique lookup capabilities by transaction id, category, etc. as are required to build this structure from the result set), but I don't think that it makes sense to reconfigure this structure for this simple use case.
There is additionally some confusion on naming of family, categories, lines between the DB and what is being output to user. For example it seems family
in DB maps to categories
for output which seem strange. You might consider aliasing your fields in your query to be able to use common terminology throughout both the section of code where you build the data structure and where you output it.
In your output, consider keeping spaces around all .
used for concatenation. This helps make code easy to read and allows for good visual line breaks in editors if a line does happen to go too long while editor is using "word wrapping".
Also, I think using objects (as mentioned in data structure comment above) instead of associative arrays helps in readability. This along with suggestion to use constants might yield code that looks like the following:
// assume you have constants S, BR, UL, LI, _UL, _LI
foreach ($data as $txn_id => $txn) {
echo $txn_id . S . $txn->date . S . $txn->source . S . $txn->amount . UL . PHP_EOL;
foreach ($txn->categories as $cat_name => $cat) {
echo LI . $cat_name . S .$cat->amount . UL . PHP_EOL;
foreach ($cat->lines as $line_name => $line) {
echo LI . $line_name . S . $line->amount . UL . PHP_EOL;
foreach ($line->items as $item_name => $item) {
echo LI . $item_name . S . $item->qty . S . $item->price . _LI . PHP_EOL;
}
echo _UL . _LI . PHP_EOL;
}
echo _UL . _LI . PHP_EOL;
}
echo _UL . BR . PHP_EOL;
}
To me, this is much easier to read and a pretty good approach short of going to more of a template approach.