0

I'm using PHP 5.2.9 at the very moment. Is there a way to refactor this code this way it's easier to read and better organized?

  if ($is_read_only == true) {
      echo ($affiliate['affiliate_gender'] == 'm') ? MALE : FEMALE;
  } elseif ($error == true) {
      if ($entry_gender_error == true) {
            echo tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE . '  ' . tep_draw_radio_field('a_gender', 'f', $female) . '  ' . FEMALE . ' ' . ENTRY_GENDER_ERROR;
      } else {
            echo ($a_gender == 'm') ? MALE : FEMALE;
            echo tep_draw_hidden_field('a_gender');
      }
  } else {
      echo tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE . '  ' . tep_draw_radio_field('a_gender', 'f', $female) . '  ' . FEMALE . ' ' . ENTRY_GENDER_TEXT;
  }
Improve this question
8
  • 2
    I assume your code is better formatted? In addition, you likely mean refactor rather than compress. Commented Nov 6, 2012 at 22:33
  • 2
    Sometimes spacing the code out makes it easier to read, the biggest help is indenting of nested sections. Commented Nov 6, 2012 at 22:34
  • 5
    Compressing generally makes it harder to read. Commented Nov 6, 2012 at 22:34
  • What gain do you wish to achieve from making the code take a smaller number of lines? Commented Nov 6, 2012 at 22:35
  • You could start by adding indentation. Commented Nov 6, 2012 at 22:38

4 Answers 4

Reset to default
4

I'm not sure why you'd want it in fewer lines, but here you go:

echo $is_read_only === true
?   $affiliate['affiliate_gender'] === 'm' ? MALE : FEMALE
:   $error === true
?   $entry_gender_error == true
?   tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE . '  ' . tep_draw_radio_field('a_gender', 'f', $female) . '  ' . FEMALE . ' ' . ENTRY_GENDER_ERROR
:   ($a_gender === 'm' ? MALE : FEMALE) . tep_draw_hidden_field('a_gender')
:   tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE . '  ' . tep_draw_radio_field('a_gender', 'f', $female) . '  ' . FEMALE . ' ' . ENTRY_GENDER_TEXT;

It surely ain't more readable. Readability and compression seem to contradict each other.

EDIT:

For the challenge in it I went a bit further.

echo $is_read_only
?   $affiliate['affiliate_gender'] === 'm' ? MALE : FEMALE
:   $error && !$entry_gender_error
?   ($a_gender === 'm' ? MALE : FEMALE) . tep_draw_hidden_field('a_gender')
:   tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE . '  ' .
    tep_draw_radio_field('a_gender', 'f', $female) . '  ' . FEMALE . ' ' .
    ($error ? ENTRY_GENDER_ERROR : ENTRY_GENDER_TEXT);

This is the worst that I, as a human, can do.

May God have mercy on my soul :)

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Oh god my eyes are bleeding! Make it stop!
LOL! I like it. Confuses the enemy! :P
+1 Truly a work of art. Succinct, Self documenting, and elegant. I am now considering all those superfluous line breaks I have been using, but at least now I have seen the light!
3

you can change if ($is_read_only == true) to if ($is_read_only) as well as your other if statement because putting '== true' is redundant and unnecessary

Improve this answer

Comments

1

It depends on what exactly you mean by "compress"?

Since you haven't clarified you're getting a basic response.

Removing spaces:

If you are expecting to speed up your code in some way, don't bother. Compressing (making smaller/removing spaces in) a php file won't speed up it's execution time. PHP reads the file every time, compiles it into bytecode and runs it. Doing this will make your eyes bleed as well as those of your colleagues. Just don't do it!

For readability/usability:

Then you'd be wise to space your code/classes/functions accordingly into blocks that make sense and are easily readable. This won't just help you but those who work alongside you. Use set indent levels, spacing/bracket/nesting styles etc.

For code performance:

There are a myriad of ways to improve code (the classes/functions/loops/connections/statements) both in visual form and for the sake of code performance - which can be profiled/tested using a wide variety of tools.

Hope this helps as a pointer.

Improve this answer

2 Comments

I apologize Nickhar, I was using the wrong terminology - basically looking at point number three (code performance) with classes/functions rather than getting lost in redundant if else if else, etc... I want to get better in improving / optimizing. Thank you for the breakdown!
In which case, I point you here for a starting point: stackoverflow.com/questions/21133/…. Plenty of questions you can ask on here about executing things this way -or- that way and people will respond!
1

I prefer it this way:

if ($is_read_only)
    echo ($affiliate['affiliate_gender'] == 'm') ? MALE : FEMALE;
elseif ($error)
    if ($entry_gender_error)
        echo tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE.
             '  ' . tep_draw_radio_field('a_gender', 'f', $female) .  
             '  ' . FEMALE . ' ' . ENTRY_GENDER_ERROR;
    else
        echo ($a_gender == 'm') ? MALE : FEMALE , tep_draw_hidden_field('a_gender');
else
    echo tep_draw_radio_field('a_gender', 'm', $male) . '  ' . MALE .
         '  ' . tep_draw_radio_field('a_gender', 'f', $female) .
         '  ' . FEMALE . ' ' . ENTRY_GENDER_TEXT;

I avoid using too long echo sentences, to improve Readability. To much {s and }s result are messy too.

Improve this answer

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.