3
\$\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\$
  • \$\begingroup\$ Try to avoid mixing PHP and HTML. See this for some advice: stackoverflow.com/questions/62617#95027 \$\endgroup\$ – Paul Aug 31 '11 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\$ – Matteo Riva Sep 5 '11 at 21:05
  • \$\begingroup\$ Yeah, opening and closing <?php is very messy, ideally you should only use that tag once. \$\endgroup\$ – Evan Carslake Apr 14 '16 at 18:12
2
\$\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\$
  • \$\begingroup\$ Minus the last ?> as it's not needed and reduces the whitespace being sent to the client. \$\endgroup\$ – bitsoflogic Aug 29 '11 at 17:31
  • \$\begingroup\$ @TobyAllen I agree with Levinaris as do others: stackoverflow.com/questions/3219383/… \$\endgroup\$ – Paul Dec 14 '11 at 2:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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