Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a form written in the usual way. After submitting, if there are errors in the form, the form page itself is the target of the redirect, with a query string appended. The query string contains the error messages, but also the values submitted with the form. The query string and its format are provided by a 3rd-party, so I cannot switch to using POST, nor make any other changes on their processing logic. I just get the string and make the best of it.

Since the URL is "different", the form's values aren't preserved. The browser treats it as a fresh form and therefore blanks it. Not good, since the user won't want to re-enter everything. So, the first step was deciding whether to use JavaScript or PHP to repopulate the form, and I went with PHP.

Consequently, the form now has entries that look a little something like this:

<li id="li_business_years" >
  <label class="description" for="partner_business_years">How many years have you been in business?</label>
  <div>
    <input id="partner_business_years" name="partner_business_years" class="element text small" type="text" maxlength="255" value="<?php echo $_GET['partner_business_years']; ?>"/>
  </div>
</li>

Is this considered a ridiculous approach? If I used JavaScript, I could use an iterator to run through all the fields in one little loop, instead of individually populating each value attribute with PHP. But server-side seems to be the right place to do this since the form doesn't otherwise rely on JavaScript.

The next question: I have some Yes/No questions in radio button form. The query string will therefore contain values like will_relocate=yes. In my mind, I want to just say "Select the appropriate radio based on the query string", but there's no real concept of selecting for radios... there's a shared name with different value options.

Here's the original HTML:

<span>
  <input id="relocate_1" name="will_relocate" class="element radio" type="radio" value="yes" />
<label class="choice">Yes</label>
</span>
<span>
  <input id="relocate_2" name="will_relocate" class="element radio" type="radio" value="no" />
  <label class="choice">No</label>
</span>

I can get it working by doing something like this:

<input id="relocate_2" name="will_relocate" class="element radio" type="radio" value="no" <?php if($_GET['will_relocate'] && $_GET['will_relocate'] == "no") { echo 'checked'; } ?>/>

(with a matching one for "yes"... and as many pairs of these as needed)

But it strikes me as hideous. I'm just a hack at PHP, and I'm sure I could break this out into a function, but I would still have a pair of <?php selectRadio('will_relocate') ?> - type calls throughout my document, which is barely better.

Any ideas? Is my approach just completely nasty from the get-go? Should I use JavaScript after all?

I'm open to JavaScript-based suggestions for form re-populating, but not as the sole validator (I don't have and wouldn't choose that option).

share|improve this question
    
Rather than unpacking the returned form contents from your PHP validator, have you considered doing client-side form validation in JavaScript? –  msanford Jun 26 '12 at 16:11
3  
I would never do client-side validation "only". It's a nice-to-have and I will probably implement it in some variety, but sanitizing on the server-side is a must-have. More to the point, though, I have no choice. If the form encounters an error on the server side, it WILL redirect to this page and I will want the form to be pre-filled. Might be a JavaScript snippet that will cookie and restore a form, though. Could be a decent compromise. –  Greg Pettit Jun 26 '12 at 16:19
    
Greg, I agree. There's always the possibility that someone might have JavaScript disabled, for example. –  msanford Jun 26 '12 at 18:24
add comment

1 Answer 1

I went ahead with the one-by-one re-populating from GET (on a timeline and all that!). The form doesn't need to be highly reusable and won't be modified very frequently after roll-out, so in this particular case, it was a matter of 'getr done' rather than 'getr done the best way'.

For the radio buttons, I ended up with a series of these:

<span>
  <input id="partner_systemIntegration_1" name="partner_systemIntegration" class="element radio" type="radio" value="yes"  <?php if(($_GET['partner_systemIntegration']) && $_GET['partner_systemIntegration'] == "yes") { echo 'checked'; } ?> />
  <label class="choice" for="partner_systemIntegration_1">Yes</label>
</span>
<span>
  <input id="partner_systemIntegration_2" name="partner_systemIntegration" class="element radio" type="radio" value="no"  <?php if(($_GET['partner_systemIntegration']) && $_GET['partner_systemIntegration'] == "no") { echo 'checked'; } ?>  />
  <label class="choice" for="partner_systemIntegration_2">No</label>
</span>

It's klunky, but when I tried to break it out into a function it failed. I'm using WordPress and a plugin that allowed PHP in the post/page content itself. When trying to define and use a function, I got some sort of eval error/warning, so I'm just doing it one at a time. Not the epitome of elegance and not my ideal choice, but now I can move on.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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