[SOLVED] Refactoring help

[wigglesby]

Hi

 

I have the following code:

 

<h3>Downloads attached to this article</h3>

	<?php $file1 = $article->getAttachedFile1() ?>
	<?php $file2 = $article->getAttachedFile2() ?>
	<?php $file3 = $article->getAttachedFile3() ?>
	<?php $file4 = $article->getAttachedFile4() ?>
	<?php $file5 = $article->getAttachedFile5() ?>

	<?php $filedesc1 = $article->getAttachedFileDesc1(); ?>
	<?php $filedesc2 = $article->getAttachedFileDesc2(); ?>
	<?php $filedesc3 = $article->getAttachedFileDesc3(); ?>
	<?php $filedesc4 = $article->getAttachedFileDesc4(); ?>
	<?php $filedesc5 = $article->getAttachedFileDesc5(); ?>

               <ul class="attached_files">
	<?php
	if(empty($file1)){
	echo '';
	}else{ ?>
	<li><a href="<?php echo '/members/files/'. $file1; ?>" >
	<?php

	$filedesc_1 = ( $filedesc1 !== '' ) ? $filedesc1  : 'Download File';
	echo $file1 .'<br />';
	?>
	</a>
	<?php echo $filedesc_1 .'<br />'; ?>

	</li>
	<?php }
	?>

	<?php
	if(empty($file2)){
	echo '';
	}else{ ?>
	<li><a href="<?php echo '/members/files/'. $file2; ?>" >
	<?php

	$filedesc_2 = ( $filedesc2 !== '' ) ? $filedesc2  : 'Download File';
	echo $file2 .'<br />';
	?>
	</a>
	<?php echo $filedesc_2 .'<br />'; ?>

	</li>
	<?php }
	?>

	<?php
	if(empty($file3)){
		echo '';
	}else{ ?>
	<li><a href="<?php echo '/members/files/'. $file3; ?>" >
	<?php

	$filedesc_3 = ( $filedesc3 !== '' ) ? $filedesc3  : 'Download File';
	echo $file3 .'<br />';
	?>
	</a>
	<?php echo $filedesc_3 .'<br />'; ?>

	</li>
	<?php }
	?>

	<?php
	if(empty($file4)){
		echo '';
	}else{ ?>
	<li><a href="<?php echo '/members/files/'. $file4; ?>" >
	<?php

	$filedesc_4 = ( $filedesc4 !== '' ) ? $filedesc4  : 'Download File';
	echo $file4 .'<br />';
	?>
	</a>
	<?php echo $filedesc_4 .'<br />'; ?>

	</li>
	<?php }
	?>

	<?php
	if(empty($file5)){
	echo '';
	}else{ ?>
	<li><a href="<?php echo '/members/files/'. $file5; ?>" >
	<?php

	$filedesc_5 = ( $filedesc5 !== '' ) ? $filedesc5  : 'Download File';
	echo $file5 .'<br />';
	?>
	</a>
	<?php echo $filedesc_5 .'<br />'; ?>

	</li>
	<?php }
	?>


</ul>

 

I want to refactor it so I dont have an endless amount of if/else clauses.

 

Can someone help me to reduce the code?

 

Thanks

[rajivgonsalves]

see if this works

 

      <h3>Downloads attached to this article</h3>
      <ul class="attached_files">
      <?php
      $limit = 5;
      for ($i=1; $i<= $limit ;$i++) {
          $file = call_user_func(array($article, 'getAttachedFile'.$i));
          $filedesc = call_user_func(array($article, 'getAttachedFileDesc'.$i));

          if(empty($file)){
            echo '';
          }
          else{
              $filedesc = ( $filedesc !== '' ) ? $filedesc  : 'Download File';

              echo '<li><a href="/members/files/'. $file . '>';
              echo $file .'<br />';
              echo '</a>;';
              echo $filedesc .'<br />';
              echo '</li>';
          }
      }
      ?>   
      </ul>

[PFMaBiSmAd]

Any time you have a SET of same type data, you need to use an array. Once your data is in the form of an array, you can use a simple for or foreach loop to iterate over the data in the set.

 

You should also extend the array concept to your getAttachedFile and getAttachedFileDesc methods so that you don't have duplicate code where the only difference is which piece of data it operates on. (i.e. your methods should probably accept a numeric parameter that causes them to return the the correct 1st through 5th values.)

[wigglesby]

Thanks guys.

 

rajivgonsalves, I have increased the limit to 10, as I have 10 file fields.

 

Using your code, only the first is displayed, and then the 4th, 6th, and 8th.

 

I need all 10 to be displayed.

 

Thanks

[rajivgonsalves]

there is a if statement

 

         if(empty($file)){
            echo '';
          }

 

did it all work fine before the refactor ?

[wigglesby]

yeah it did

[rajivgonsalves]

try this I modified some code

 

   <h3>Downloads attached to this article</h3>
      <ul class="attached_files">
      <?php
      $limit = 5;
      for ($i=1; $i<= $limit ;$i++) {
          $file = call_user_func(array($article, 'getAttachedFile'.$i));
          $filedesc = call_user_func(array($article, 'getAttachedFileDesc'.$i));

          if(empty($file)){
            echo '';
          }
          else{
              $filedesc = ( $filedesc !== '' ) ? $filedesc  : 'Download File';
              echo "<li><a href=\"/members/files/{$file}\">{$file}</a><br />{$filedesc}<br /></li>";
          }
      }
      ?>   
      </ul>

[wigglesby]

That seems to work perfectly!!

 

Thank you