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;
  }
klewis
  • 7,459
  • 15
  • 58
  • 102
  • 2
    I assume your code is better formatted? In addition, you likely mean *refactor* rather than *compress*. – Jason McCreary Nov 06 '12 at 22:33
  • 2
    Sometimes spacing the code out makes it easier to read, the biggest help is indenting of nested sections. – Scuzzy Nov 06 '12 at 22:34
  • 5
    Compressing generally makes it harder to read. – Peter Gluck Nov 06 '12 at 22:34
  • What gain do you wish to achieve from making the code take a smaller number of lines? – sean Nov 06 '12 at 22:35
  • You could start by adding indentation. – Wug Nov 06 '12 at 22:38
  • You can put your string concatenations on separate lines; `FEMALE . [newline] ' ' . [newline] ENTRY_GENDER_ERROR;` (where `[newline]` is an actual carriage return, obviously). Make code take up less space generally makes it *harder*, not easier, to read. – Kelvin Nov 06 '12 at 22:40
  • You have to choose one .. Compression or Easier to Read .. it would also be better for you to upgrade your PHP version – Baba Nov 06 '12 at 22:58
  • Yes Jason, I apologize. I am still learning proper terminology here. Yes I mean refactor, not compress. I feel dumb :) – klewis Nov 07 '12 at 02:37

4 Answers4

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 :)

iMoses
  • 4,338
  • 1
  • 24
  • 39
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

Yellowledbet
  • 147
  • 3
  • 9
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.

Reger
  • 474
  • 4
  • 17
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.

nickhar
  • 19,981
  • 12
  • 60
  • 73
  • 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! – klewis Nov 07 '12 at 02:40
  • In which case, I point you here for a starting point: http://stackoverflow.com/questions/21133/simplest-way-to-profile-a-php-script. Plenty of questions you can ask on here about executing things this way -or- that way and people will respond! – nickhar Nov 07 '12 at 02:45