[Webinar] Streamline your web hosting managementRegister Today

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 116
  • Last Modified:

How can I convert this switch / case dynamic into a regular if / else

I need to reconfigure this code and I want to replace the switch / case to a if / else. If all goes well, there will probably be some more questions forthcoming and you'll see why.

Thing is, I'm missing something and I need some other eyes on it. Here's the code and the corresponding page:

<li class='event <?=$left?'left':'right'?> <?=$eventtype?>' data-type='<?=$eventtype=='transactions'?'transaction':$eventtype?>'>
<div class='ev_container'>
    <div class='event_header'><span class='event_icon'><span class='appicons <?=strtolower($eventtype)?>'></span></span><?=ucfirst($eventtype)?></div>
    <div class='event_body'>
      <div class='content'>
        <span class='arrow'><span class="point"><span class="timeline_stopper top"><span></span></span><span class="circle"></span><span class="timeline_stopper bottom"><span></span></span></span></span></span>
      <?
      switch($eventtype){
        case "comment":
        case "text":
        case "note":
        ?>
          <?=$content?>
        <?
          break;
        case "statement":
            $dir_exists = $demo ? true : is_dir($dir);
            $file_exists = $demo ? true : file_exists($dir . $file);

            if($file_exists && $dir_exists) {
        ?>
            <a href='<?=$demo?'/admin/statement/?accountid='.$demo:'/admin/statement/load/'.$content?>' target='_statement'><img src='/assets/images/pdf_hover.png'/>View Statement</a>
        <?
            } else if($dir_exists) {
        ?>  
            <span class="error">Failed to send.</span>
        <? 
            } else {
        ?> 
            <span class="pending">Attempted to send... Pending feedback.</span>
        <?
            }
          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>
        <?}?>
        </ul><?
      }
  ?></div>
    <?if(isset($runningbal) && ($new_eventtype == 'transaction' || $eventtype == 'transactions')){?>
    <div class='runningbal'>Running balance : <span><?=$runningbal?></span></div>
    <?}?>
    <div class='datefoot'>
      <span class='appicons clock'></span><?=date('M d, Y'.($eventtype!='transactions'?' / h:i A':''),strtotime($eventdate))?>
    </div>
  </div>
</div></li>

Open in new window


current page
Here's my attempt to replace the switch / case with a regular if /else and the epic fail that results:

<li class='event <?=$left?'left':'right'?> <?=$new_eventtype?>' data-type='<?=$new_eventtype=='transactions'?'transaction':$new_eventtype?>'>
<div class='ev_container'>
    <div class='event_header'><span class='event_icon'><span class='appicons <?=strtolower($eventtype)?>'></span></span><?=ucfirst($eventtype)?></div>
    <div class='event_body'>
      <div class='content'>
        <span class='arrow'><span class="point"><span class="timeline_stopper top"><span></span></span><span class="circle"></span><span class="timeline_stopper bottom"><span></span></span></span></span></span>
      <?
	  if($eventtype=="comment" OR $eventtype=="text" OR $eventtype=="note")
	  {
		echo $content;
	  }
		if($eventtype=="statement")
		{
            $dir_exists = $demo ? true : is_dir($dir);
            $file_exists = $demo ? true : file_exists($dir . $file);

            if($file_exists && $dir_exists) {
        ?>
            <a href='<?=$demo?'/admin/statement/?accountid='.$demo:'/admin/statement/load/'.$content?>' target='_statement'><img src='/assets/images/pdf_hover.png'/>View Statement</a>
        <?
            } else if($dir_exists) {
        ?>  
            <span class="error">Failed to send.</span>
        <? 
            } else {
        ?> 
            <span class="pending">Attempted to send... Pending feedback.</span>
        <?
            }
		}
		if($eventtype=="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>
		<?php
		}
		?>
			<?
			if($eventtype=="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>
			<?}?>
			</ul><?
			}
  ?></div>
    <?if(isset($runningbal) && ($eventtype == 'transaction' || $eventtype == 'transactions')){?>
    <div class='runningbal'>Running balance : <span><?=$runningbal?></span></div>
    <?}?>
    <div class='datefoot'>
      <span class='appicons clock'></span><?=date('M d, Y'.($eventtype!='transactions'?' / h:i A':''),strtotime($eventdate))?>
    </div>
  </div>
</div></li>

Open in new window


Here's the page:

my page
You'll notice with my page, there are no icons. It's just a running list and I need to duplicate the look of the original page.

What am I missing?
0
brucegust
Asked:
brucegust
4 Solutions
 
gr8gonzoConsultantCommented:
Compare the HTML output between the two versions. Chances are that you're missing some closing </div> or </span> or something, resulting in incorrect styling.
1
 
Ray PaseurCommented:
In addition to gr8gonzo's excellent advice, I recommend you stop using the short-open tags forever.  This is a settable configuration option that you cannot always depend on.  This can only lead to sadness and tears, and is regrettable since the full start-php tag is always available.

There's almost never any value in making switch / case into the more verbose if / else.  But that aside, please have a look at this awkward piece of code:
      switch($eventtype){
        case "comment":
        case "text":
        case "note":
        ?>
          <?=$content?>
        <?
          break;

Open in new window

Here is how a professional might express it:
      switch($eventtype){
        case "comment":
        case "text":
        case "note":
          echo $content;
          break;

Open in new window

I would recommend refactoring this code.  It's a spaghetti-mess and will prove (as you are now seeing) very difficult to maintain.
1
 
Julian HansenCommented:
The only real difference between the files is line 69 / 70 respectively
    <?if(isset($runningbal) && ($new_eventtype == 'transaction' || $eventtype == 'transactions')){?>

Open in new window

    <?if(isset($runningbal) && ($eventtype == 'transaction' || $eventtype == 'transactions')){?>

Open in new window


$new_eventtype vs $eventtype - can't find the definition of $new_eventtype.
1
The 14th Annual Expert Award Winners

The results are in! Meet the top members of our 2017 Expert Awards. Congratulations to all who qualified!

 
Julian HansenCommented:
Note: converting switch to if you should use else on the subsequent if statements. The breaks in the case statement effectively terminate processing of the case so no need to test the other cases.

As a matter of interest - why are you converting from one to the other.

If the above don't help - can you post the rendered output of both pages.
0
 
Brian TaoSenior Business Solutions ConsultantCommented:
One other thing, at line #8 of your new page:
if($eventtype=="comment" OR $eventtype=="text" OR $eventtype=="note")

Open in new window

should be
if($eventtype=="comment" || $eventtype=="text" || $eventtype=="note")

Open in new window

0
 
Julian HansenCommented:
@Brian
PHP supports both the double pipe (||) and the OR operator - however they operate at different precedence. The Word version (OR) has a lower operator precedence than ||

In this case it makes no difference as there are no other operators in the expression.
3
 
brucegustPHP DeveloperAuthor Commented:
Gentlemen! Forgive me for not being more timely in closing / resolving this question. The good news is that today I turned the completed project, as far as adding an additional feature to the code that you see. Rather than trying to revamp the "switch" dynamic, I just chose to fix something that wasn't broke and kept the format the same. Initially I hesitated only because I'm not as familiar with the swtich / break approach and I was looking for ways to break things down into what was most familiar. Once I got my head around the real heavy lifting, however, I was able to move forward even with being a little guarded about the switch syntax. As always, thanks for your time and your help!
0
 
Julian HansenCommented:
Welcome as always Bruce,
0

Featured Post

Free Tool: Subnet Calculator

The subnet calculator helps you design networks by taking an IP address and network mask and returning information such as network, broadcast address, and host range.

One of a set of tools we're offering as a way of saying thank you for being a part of the community.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now