4
\$\begingroup\$

This is a page that allow user to do register:

<?php echo form_open('user/valid_register_page', 'autocomplete="off"');?>

    <?php $this->table->set_template(array('table_open'=>'<table class="table_form_register">')); 
    $this->table->add_row(form_label($label_email.
                                    form_label(form_error('email'),'',array('class' => 'error_label')), 
                                    'label_email',
                                     array('class' => 'form_label')), 

                           array('class' => 'align_right_td', 
                                 'data' => form_input('email', 
                                           set_value('email'),
                                           'class = "align_right_input require"')));  

    $this->table->add_row(form_label($label_invitation_key.
                                    form_label(form_error('invitation_key'),'',array('class' => 'error_label')),
                                    'label_invitation_key',
                                     array('class' => 'form_label')),

                          array('class' => 'align_right_td',  
                                 'data' => form_password('invitation_key',
                                           set_value('invitation_key'),
                                           'class = "align_right_input require"')));

    $this->table->add_row('',array('class' => 'button_td',
                  'data' => form_submit('register',  $button_register, 'class = "form_td_button"')));

    echo $this->table->generate();    

    ?>          
<?php echo form_close();?>  

This code only have 2 label and 2 fields: one is email, another is the invitation keys. This is the code that render for generation the form, table, and the button with these 2 label and fields. But as you can see, it is very messy, if I need to maintain, it will become a headache. Any suggestions on how to make it simpler?

\$\endgroup\$
3
  • \$\begingroup\$ Try to avoid mixing PHP and HTML. See this for some advice: stackoverflow.com/questions/62617#95027 \$\endgroup\$
    – Paul
    Aug 31, 2011 at 15:32
  • 1
    \$\begingroup\$ I really don't get these "HTML helpers". The name suggests they should be helping, while all I see is an incredibly verbose way to write what would be a lot more readable as plain HTML. \$\endgroup\$ Sep 5, 2011 at 21:05
  • \$\begingroup\$ Yeah, opening and closing <?php is very messy, ideally you should only use that tag once. \$\endgroup\$ Apr 14, 2016 at 18:12

3 Answers 3

3
\$\begingroup\$

You could start by removing dynamic HTML generation where there is no benefit in it. You would be much better of writing something like this :

<?php
   $form = $this->form;
?>
<form action="<?php echo $form->get_destination(); ?>" method="post">
    <ul>
        <li>
            <?php echo $form->render_field( 'email' ); ?>
        </li>
        <li>
            <?php echo $form->render_field( 'invitation_key' ); ?>
        </li>
        <li><?php echo $form->render_field( 'submit' ); ?></li>
    </ul>
</form>

Yes, none of it is CodeIgniter , but then again i have huge dislike for frameworks with crappy OOP.

\$\endgroup\$
1
\$\begingroup\$

No need to open and close php tags over and over. just open them at the beginning, then close them at the end.

Also, you're stringing array after array in there. create variables to hold the arrays and create them before they're needed. Then, you can use them in the add_row calls. This will clean up most of the code.

\$\endgroup\$
1
\$\begingroup\$

This seems to me to be one single piece of php so therefore you should put all the code above into a single

<?php
//Put your php code here.

Use indentation correctly and consistently.

The general consensus for use of end php brakets ?> is that you do not include it at the end of the file. PHP does this automatically, and it avoids certain hard to find bugs.

\$\endgroup\$
2
  • \$\begingroup\$ Minus the last ?> as it's not needed and reduces the whitespace being sent to the client. \$\endgroup\$ Aug 29, 2011 at 17:31
  • \$\begingroup\$ @TobyAllen I agree with Levinaris as do others: stackoverflow.com/questions/3219383/… \$\endgroup\$
    – Paul
    Dec 14, 2011 at 2:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.