Solved

How does this "for" loop break down (Part II)?

Posted on 2016-08-30
7
39 Views
Last Modified: 2016-09-02
This is "Part II" of a two part question. Part I is at https://www.experts-exchange.com/questions/28966558/How-does-this-for-loop-break-down-Part-I.html. I've got a couple of graphics that provide some background that you might want to refer to.

We're at line 11 of the following code:

$data = pf::getAppData('/app/accounttimeline');
$timeline = AccountTimelines::getAccountTimeline($accountid); //the SELECT statement that produces the sample data I've got listed above
$data['timeline'] = array();
$txnGroup = array();
 for ($i=0; $i < count($timeline); $i++) {
	$event = $timeline[$i];
	$event['demo'] = ($data['badge']['practiceid'] == '63'?$accountid : false);
	if ($event['eventtype']=='transaction') {
		$thisDate = Date('mdy', strToTime($event['eventdate']));
		$nextDate = (isset($timeline[$i+1])&&$timeline[$i+1]['eventtype']=='transaction')?Date('mdy', strToTime($timeline[$i+1]['eventdate'])):null;
		array_push($txnGroup, $event);
		if ($thisDate != $nextDate) {
			//add to data
			if (count($txnGroup)!=1) {
				array_push($data['timeline'], array(
					'eventdate'=>Date('m/d/Y', strToTime($event['eventdate'])),
					'eventtype'=>'transactions',
					'runningbal'=>$event['runningbal'],
					'transactions'=>$txnGroup
				));
			} else {
				array_push($data['timeline'], $event);
			}
			$txnGroup = array();
		}

Open in new window


Line 11 is crucial to the project that I'm working on. We've got a multi-dimensional array that's being enhanced with the data coming from a SELECT statement. We're setting up some additional indices to hold more data and we're a point now where we're checking the eventtype to see if it's a "transaction:"

if ($event['eventtype']=='transaction') {//if the eventtype is a "transaction," then...
		$thisDate = Date('mdy', strToTime($event['eventdate'])); //set up $thisDate as a strtotime variable
		$nextDate = (isset($timeline[$i+1])&&$timeline[$i+1]['eventtype']=='transaction')?Date('mdy', strToTime($timeline[$i+1]['eventdate'])):null;//[b]if the next eventtype in the recordset is also a "transaction," then...not sure what this is doing[/b]
		array_push($txnGroup, $event);// you're using array_push to add everything that's currently compiled in the $event index to the $txnGroup array
 if ($thisDate != $nextDate) {//[b]if $thisData is not equal to $nextDate, then you've got a potential group of transactions as opposed to something that's flying solo[/b]
                        //add to data
                        if (count($txnGroup)!=1) {//if the there's more than one transaction in the $txnGroup, then we're going to make the eventtype "transactions," rather than "transaction"
                            array_push($data['timeline'], array(//push all of the following into the $txnGroup array
                                'eventdate'=>Date('m/d/Y', strToTime($event['eventdate'])),
                                'eventtype'=>'transactions',
                                'runningbal'=>$event['runningbal'],
                                'transactions'=>$txnGroup
                            ));
                        }

Open in new window


This is the last part of the "for" loop, but it represents something that I'm suspecting, but I'm not certain about...

}else {//if the eventtype is not a "transaction," then simply push what's there into the $event
array_push($data['timeline'], $event);
 }
 $txnGroup = array();//I think what's happening here is that I'm resetting the $txnGroup array to "". Rather than it being populated with anything, I'm starting fresh with the next round of $I in my "for" loop

Open in new window


Am I close?
0
Comment
Question by:brucegust
  • 3
  • 3
7 Comments
 
LVL 108

Assisted Solution

by:Ray Paseur
Ray Paseur earned 50 total points
ID: 41776697
Well, this sort of code is kind of an object lesson in why professional developers use object-oriented design and programming.  For more understanding of why this spaghetti-mess is a problem, read up on cyclomatic complexity and the painful difficulty of proving whether a complex program can ever be free of bugs.

In this case, I think your comments in the last code snippet are probably correct, but I can't say for sure without seeing the rest of the for() loop.  If other things are happening below $txnGroup = array(), then all bets are off until we see that code, too.

In the second code snippet, this might  cause a parse error (not sure, never saw anyone write things like this)
array_push($data['timeline'], array(//push all of the following into the $txnGroup array

Here's how array_push() works:
http://php.net/manual/en/function.array-push.php

Push  and pop  are terms of art in computer programming, used in stack and queue management.
0
 
LVL 33

Expert Comment

by:Slick812
ID: 41776919
greetings  brucegust , , , In looking at your code with your comments, , I do not believe u are seeing what this code tries to do.  It is true that the only "IF TEST" you have in this for the $event "eventtype" is the value of  'transaction'  as this line shows -
               if ($event['eventtype']=='transaction') {

but in many data SELECTS there are times that some ROWS need to be grouped or marked as being similar, and this code tries to use the  'eventdate'     to set an EXTRA array entry when the date changes detected by this line -
          if ($thisDate != $nextDate) {

and these lines get the current event date and the next SELECT ROW  event date -
       $thisDate = Date('mdy', strToTime($event['eventdate']));
       $nextDate = Date('mdy', strToTime($timeline[$i+1]['eventdate']));    // shortened code

so if the two ROWS eventdate are the same then it just adds the event to the array
    $data['timeline'][] = $event;

if the dates are different in the   -
     if ($thisDate != $nextDate) {

then this array is placed in the $data['timeline'][ ]  - --
    array( 'eventdate'=>Date('m/d/Y', strToTime($event['eventdate'])),
      'eventtype'=>'transactions',
      'runningbal'=>$event['runningbal'],
      'transactions'=>$txnGroup
      )

as a MARKER to show that the previous added events are grouped by DATE, all of the $event  with the same dates are in the  $txnGroup, in the  'transactions' of the added array

SOOOO, then the if test for -
         if ($thisDate != $nextDate) {
is TRUE then the  $txnGroup  array is emptied with -
     $txnGroup = array();

and the next data ROWS as $event are added to the $txnGroup until the date is not equal.

Hope this helps some how? but you failed to place the end    }    for the -
          for ($i=0; $i < count($timeline);    ) {
LOOP, and the
          if ($event['eventtype']=='transaction') {

So there may be other operations below the code you have shown. . .

- - - - - - - - -
I must say that almost NO DEVELOPERS in PHP use your  array_push( ) , instead they use the common array  [ ]   as -
      ///  array_push($data['timeline'], $event); // not good to use
    $data['timeline'][ ] = $event; // a more common PHP usage.

But maybe this code was "Translated" from another coding language ? ?
0
 

Author Comment

by:brucegust
ID: 41778179
Gentlemen!

As a quick aside, I've got a buddy who's taking some classes in an effort to become a PHP developer. I spend far too much time asking questions to feel like an "authority," but he seeks me out nevertheless for some help with some basic code.

He's taking a class in Java and he asked me if I could help. I know nothing about Java, but I encouraged him to subscribe to EE if for no other reason than with Google you can get the WHAT, but you very rarely get the WHY. And when you're seeking to understand something and not just get something done, EE is an invaluable resource and this question reinforces that point.

Thank you!

Slick, I've posed several questions, as Ray can attest, pertaining to this project that demonstrates several fallacies when it comes to healthy code. The client is migrating everything over to a third party software that promises to meet their needs and my role is to simply plug the holes in their current app and add some additional features until they've moved everyone over to the new system.

It's a bear. But... it's a healthy exercise in stretching my knowledge and it's resources such as yourself, Ray and several others, who keep things moving forward as opposed to getting stuck.

With that said:

I'm tracking with you on everything with the exception of the MARKER.

When I think of an array, rather than a series of rows, I'm thinking of a single row with multiple columns. It sounds as though the ONLY time "transactions" shows up as a value within the eventtype column is in the aftermath of a series of rows ($event being the index) with the same date.

Take a look at some sample data as it's coming from the database:

sample data
So, you've got several rows in the database that have the same date. As it's being processed by the above code, I would THINK that those rows that have the same date would be categorized as "transactions," yes?

On the page that's actually displaying the data, I see this:

  break;
        case "transaction":
        ?>
        <div class='amount'><?=GlobalFunctions::formatMoney($amount)?><span class='float'>
          <span class='payer <?=$payer?>'><?=$payer?></span><span class='transtype <?=$transactiontype?>'><?=$transactiontype?></span>
        </div>
        <?if($dos !== null && $dos !=='1900-01-01'){?><div class="dos"><?='DOS : ' . date('M d, Y',strtotime($dos))?></div><?}?>
        <div class='encounter'><?if($encountercode != null){?>Encounter Code <?=$encountercode?><?}else{?>No encounter<?}?></div>
        <div class='description'>
          <?=$content?>
        </div>
        <?
          break;
        case "transactions":
        ?>
        <ul>
        <?foreach($transactions as $txn){
        ?>
        <li>
          <div class='amount'><?=GlobalFunctions::formatMoney($txn['amount'])?><span class='float'>
            <span class='payer <?=$txn['payer']?>'><?=$txn['payer']?></span><span class='transtype <?=$txn['transactiontype']?>'><?=$txn['transactiontype']?></span>
          </div>
          <?if($txn['dos'] !== null && $txn['dos'] !=='1900-01-01'){?><div class="dos"><?='DOS : ' . date('M d, Y',strtotime($txn['dos']))?></div><?}?>
          <div class='encounter'><?if($txn['encountercode'] != null){?>Encounter Code <?=$txn['encountercode']?><?}else{?>No encounter<?}?></div>
          <div class='description'>
            <?=$txn['content']?>
          </div>
          <div class='txntime'><?=Date('h:i A',strtotime($txn['eventdate']))?></div>
          <?/*if(isset($txn['runningbal'])){?>
           <div class='runningbal'>Running balance : <span><?=$txn['runningbal']?></span></div>
          <?}*/?>
        </li>
        <?}

Open in new window


On line 2, you have "transaction," which would be a single row. But then on line #14, you've got "transactions." A few lines down, you've got a "foreach," which makes perfect sense in that you've got several rows worth of data, each having the same date.

But I'm thinking that every one of those rows that's getting ready to be spit out by that "foreach" would have to have an eventtype of "transactions," and it sounds like, from your explanation, "transactions" only shows up once - kind of like an exclamation point - after a series of rows with the same date.

That doesn't sound right. Where am I falling short in my understanding?
0
Top 6 Sources for Identifying Threat Actor TTPs

Understanding your enemy is essential. These six sources will help you identify the most popular threat actor tactics, techniques, and procedures (TTPs).

 
LVL 33

Expert Comment

by:Slick812
ID: 41778358
You ask - "As it's being processed by the above code, I would THINK that those rows that have the same date would be categorized as "transactions," yes? "

This is the Idea, but Not to close to the "Code" logic processing, , as you have a TEST of some variable in a PHP Switch and Case operation, and the OUTPUT display Changes with different matches in the   CASE:



OK, you did NOT seem to notice that the TWO case types of -
       case "transaction":
       case "transactions":

give DIFFERENT HTML groupings as the "transaction" case will do 4 <div> output, ,
BUT the "transactions" do an HTML LIST, that begins with a <ul> , and then a <li> is added for each of the $event that was added to the $txnGroup array in the PREVIOUS PHP building of arrays. Each <li> has 5 <div> output, very similar to the "transaction" case HTML output. Please consider WHY development staff wants to show a list <ul> grouping for time-date related data rows.

- - - - - - - - - - - - - - - - - -  - - - -
You say - "But I'm thinking that every one of those rows that's getting ready to be spit out by that "foreach" would have to have an eventtype of "transactions," and it sounds like, from your explanation, "transactions" only shows up once - kind of like an exclamation point - after a series of rows with the same date. "

Yess to the Last part, ,  an exclamation point is a MARKER in english writing, that signifies the END of sentence, and Special Importance for that sentence. BUT you do NOT see what is going on in the building of the $data['timeline'] Array used for OUTPUT with the SWITCH-CASE ,

You say - "I'm thinking that every one of those rows that's getting ready to be spit out by that "foreach" would have to have an eventtype of "transactions," ", , , , , but this is not what can work, because the OUTPUT has to be changed to a list <ul> for grouping for an INNER array (not the $data['timeline'] array) that has all of the stored $event from the  $txnGroup array, all of the $event (a data ROW) is in the $transactions array in your SWITCH-CASE code, and are NOT tested for their eventtype,
the added ARRAY entry in the build code as -
array_push($data['timeline'], 
  array('eventdate'=>Date('m/d/Y', strToTime($event['eventdate'])),
  'eventtype'=>'transactions',
  'runningbal'=>$event['runningbal'],
  'transactions'=>$txnGroup
  ));

Open in new window


is tested for the 'eventtype', BUT in that array there is sub-array, the -
    'transactions'=>$txnGroup
which is what operates in the OUTPUT CASE as the foreach -
<?foreach($transactions as $txn){ ?>

, , I am not trying to comment on how this code, to me, seems inefficient and CONFUSING. BUT the developers should NEVER have used the database table  ROW field name  "eventtype" value as  'transaction' and  'transactions'  as the $data['timeline'] eventtype, , it just TOO CONFUSING, they would do better at use eventtype value as "single" and "dategroup", or some other. the image you gave for SELECT results has all eventtypes as "transaction", so I wonder what happens if in the database SELECT a eventtypes is NOT a "transaction"? ? ?

I have tried to get you to re analyze the code operations in your presentation here, so you have a better understanding of the "WHY" this was set as the code used to get a database read, and then output to HTML with PHP to have a display with sections and groupings for many ROWS of data.
0
 

Author Comment

by:brucegust
ID: 41780850
Slick!

I built a "sandbox" in order to better isolate the code and view the results.

I was puzzled as to why the MARKER would come after the rows that had been identified as a group. But then I saw the last part of the function: $data['timeline'] = array_reverse($data['timeline']); and it made perfect sense. We're going in reverse order so putting the MARKER at the tail end of the grouped variables will work when it's run from the back to the front.

I get it!

But I wanted you to see this.

Here's my code:

		 $set = AccountTimelines::getAccountTimeline(11128265);
		 
		 //var_dump($timeline);
		   $runningbal = 0;
		 foreach($set as &$item){
		  if($item['eventtype'] == 'transaction'){
			$runningbal += pfmoney::toCents($item['amount']);
			$item['runningbal'] = pfmoney::fromCents($runningbal);
		  }
		} //adding the runningbal index to the array if the eventtype is a transaction
		
		 $data['timeline'] = array();
		 $txnGroup = array();
		 
		  for ($i=0; $i < count($set); $i++) {
                $event = $set[$i]; //changing what would otherwise be a cumbersome key to something a little more intuitive 
				if($event['eventtype']=='transaction')
				{
					$thisDate = Date('mdy', strToTime($event['eventdate']));
					$nextDate = (isset($timeline[$i+1])&&$timeline[$i+1]['eventtype']=='transaction' AND $timeline[$i+1]['payer']=='I' AND $timeline[$i+1]['transactiontype']=='P' )?Date('mdy', strToTime($timeline[$i+1]['eventdate'])):null;
					array_push($txnGroup, $event); //pushing anything with an eventtype of "transaction" to its own array
					if ($thisDate != $nextDate) //looking to see if the next row has a date that is not equal to the current row's date. If not, then
					{
						//add to data
						if (count($txnGroup)!=1) 
						{
							array_push($data['timeline'], array(
								'eventdate'=>Date('m/d/Y', strToTime($event['eventdate'])),
								'eventtype'=>'transactions',
								'runningbal'=>$event['runningbal'],
								'transactions'=>$txnGroup//I don't understand this
							));
						} 
						else 
						{
							array_push($data['timeline'], $event); //anything that's not a transaction event type gets put into the $data['timeline'] array as a regular $event
						}
					}
				}
		  }
		  
		  //var_dump($data['timeline']);
		  
		 foreach($data['timeline'] as $txn)
		  {
			  echo $txn['transactions'].'<br>';
		  }

Open in new window


And here's a screen shot of what I get in my sandbox as well as what I get when I run the actual page.

Sandbox...

sandbox
Actual page...

actual page
Why do I get the error in my sandbox, and then get the "arrays." I'm thinking the arrays should the 'transactions'=>$txnGroup//I don't understand this" part of the code.

Bottom line: The new array that serves as a marker makes total sense now that I've been able to understand the flow of the data (reversed), but what's the significance of the "transactions" index? That last line: "transactions=?$txnGroup" is the last piece of the puzzle.

What do you think?

Thanks!
0
 
LVL 33

Accepted Solution

by:
Slick812 earned 450 total points
ID: 41780915
I am really puzzled by the idea (conclusion) you have made about the -
    $txn['transactions']
as you have already shown code for this -
      'transactions'=>$txnGroup

in the addition of the array -
        array_push($data['timeline'], array(
           'eventdate'=>Date('m/d/Y', strToTime($event['eventdate'])),
           'eventtype'=>'transactions',
           'runningbal'=>$event['runningbal'],
// the $txnGroup below is an ARRAY, that is built from $events with the same DATE
           'transactions'=>$txnGroup
           ));

But I tried to EXPLAIN what the SAME DATE in the events did as I said -
    "so if the two ROWS eventdate are the same then it just adds the event to the array
        $data['timeline'][] = $event; "

although you still are using the BAD CODE for PHP as array_push( ) with this bug eyed line -
        array_push($txnGroup, $event); // should be   $txnGroup[ ] = $event;
      //pushing anything with an eventtype of "transaction" to its own array

your comment says that if the "eventtype is a "transaction" it's pushed into array, but that's NOT what matters, the eventtype is NOT the MAIN consideration here, the MAIN thing  is the DATE in   $event['eventdate'] being the SAME  for all events, to make a GROUP list with a HTML <ul><li> with all same DATE in the <li> lists,

The   $txnGroup  keeps getting events added to that Array, as LONG as the dates are the same, and when the date changes (the next ROW date is NOT the same) this line returns true -
     if ($thisDate != $nextDate) {
and the   $txnGroup   Array  is placed in the array that is added to the  $data['timeline']  Array.

So you can have more Correct html output in your Sandbox development CHECKING output ,  with this -
foreach($data['timeline'] as $txn)  echo 'Row eDate: '.$txn['transactions']['eventdate'].'<br>';
		  

Open in new window

you Know that all of the arrays in the $txn['transactions'] array are the $event DB ROWS, , so you Must Use the Table column names as the second Array parameter, I use the date column and add  ['eventdate']
but you could have used  ['amount']     or  ['content']  or any other column name. . . . .

I hope you take in some of what I explain.

For your own thinking, , please try and figure out, what you would do in development, if you had a similar SELECT , and wanted to group all of the rows with same date, in the HTML output, with color, with list or with div border, to show a Group. Obviously you would have to test and compare the "date" column for two adjacent ROWS. Would it be more efficient to compare the current Row date, with the Previous Row date, or the Next Row date? ? Could you just add the list-color to a changed date in the Select row fetch WHILE loop, OR would you need to make a separate Array of Rows (as they did here), to collect all rows of same date together? ?
0
 

Author Closing Comment

by:brucegust
ID: 41781442
Slick, you've written a book for me, man! Thank you! Ray...as always, it's a treat. Consider yourselves the recipient of a digital Diet Coke on me! Rock on and Happy Friday!
0

Featured Post

Find Ransomware Secrets With All-Source Analysis

Ransomware has become a major concern for organizations; its prevalence has grown due to past successes achieved by threat actors. While each ransomware variant is different, we’ve seen some common tactics and trends used among the authors of the malware.

Join & Write a Comment

This article will explain how to display the first page of your Microsoft Word documents (e.g. .doc, .docx, etc...) as images in a web page programatically. I have scoured the web on a way to do this unsuccessfully. The goal is to produce something …
This article discusses how to create an extensible mechanism for linked drop downs.
The viewer will learn how to count occurrences of each item in an array.
The viewer will learn how to create and use a small PHP class to apply a watermark to an image. This video shows the viewer the setup for the PHP watermark as well as important coding language. Continue to Part 2 to learn the core code used in creat…

705 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

18 Experts available now in Live!

Get 1:1 Help Now