Skip to main content
about the misuse of empty()
Source Link
mickmackusa
  • 8.7k
  • 1
  • 16
  • 31
  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.

    Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.

  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).

    file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).

  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.

    Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.

  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.

    Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.

  5. Don't repeat the same label. More generally, don't repeat yourself.

    Don't repeat the same label. More generally, don't repeat yourself.

  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.

    I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.

  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?

    I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?

  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.

    maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.

  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.

    There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.

  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.

    Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.

  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.

    I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.

  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

    Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

  13. It's no longer in my recommended script, but I do not advise the use of empty() unless your script actually needs to 1. check if the variable is declared and 2. check if the variable has a falsey value. In your script, you called trim() before calling !empty(); it could have been:

    while (!feof($file_handle)) {
        $line = trim(fgets($file_handle));
        if ($line) { // function-less truthy check is the same
            // ...
        }
    }
    

    Looking closer, I don't like that you are accessing/iterating the file contents twice. This is even more reason that I will urge you to adopt my refactored snippet. continue is certainly a useful tool, but in your code, it can be refactored away.

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.
  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.

  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).

  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.

  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.

  5. Don't repeat the same label. More generally, don't repeat yourself.

  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.

  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?

  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.

  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.

  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.

  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.

  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

  13. It's no longer in my recommended script, but I do not advise the use of empty() unless your script actually needs to 1. check if the variable is declared and 2. check if the variable has a falsey value. In your script, you called trim() before calling !empty(); it could have been:

    while (!feof($file_handle)) {
        $line = trim(fgets($file_handle));
        if ($line) { // function-less truthy check is the same
            // ...
        }
    }
    

    Looking closer, I don't like that you are accessing/iterating the file contents twice. This is even more reason that I will urge you to adopt my refactored snippet. continue is certainly a useful tool, but in your code, it can be refactored away.

edited body
Source Link
mickmackusa
  • 8.7k
  • 1
  • 16
  • 31

The way I see it, your whole process can be boiled down to this:

<label for="country">** Select Country: </label>
<?php
$countries = file('countries.txt', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!$countries) {
    echo '<input type="text" name="country" id="country" maxlength="40" size="40">';
} else {
    array_unshift($countries, '');
    printf(
        '<select name="country" id="country"><option>' . "\n\t%s</option>\n</select>",
        implode("</option>\n\t<option>", $countries)
    );
}

AdviseAdvice:

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

The way I see it, your whole process can be boiled down to this:

<label for="country">** Select Country: </label>
<?php
$countries = file('countries.txt', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!$countries) {
    echo '<input type="text" name="country" id="country" maxlength="40" size="40">';
} else {
    array_unshift($countries, '');
    printf(
        '<select name="country" id="country"><option>' . "\n\t%s</option>\n</select>",
        implode("</option>\n\t<option>", $countries)
    );
}

Advise:

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

The way I see it, your whole process can be boiled down to this:

<label for="country">** Select Country: </label>
<?php
$countries = file('countries.txt', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!$countries) {
    echo '<input type="text" name="country" id="country" maxlength="40" size="40">';
} else {
    array_unshift($countries, '');
    printf(
        '<select name="country" id="country"><option>' . "\n\t%s</option>\n</select>",
        implode("</option>\n\t<option>", $countries)
    );
}

Advice:

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.
added 303 characters in body
Source Link
mickmackusa
  • 8.7k
  • 1
  • 16
  • 31

The way I see it, your whole process can be boiled down to this:

<label for="country">** Select Country: </label>
<?php
$countries = file('countries.txt', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!$countries) {
    echo '<input type="text" name="country" id="country" maxlength="40" size="40">';
} else {
    array_unshift($countries, '');
    printf(
        '<select name="country" id="country"><option>' . "\n\t%s</option>\n</select>",
        implode("</option>\n\t<option>", $countries)
    );
}

Advise:

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

The way I see it, your whole process can be boiled down to this:

<label for="country">** Select Country: </label>
<?php
$countries = file('countries.txt', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!$countries) {
    echo '<input type="text" name="country" id="country" maxlength="40" size="40">';
} else {
    array_unshift($countries, '');
    printf(
        '<select name="country" id="country"><option>' . "\n\t%s</option>\n</select>",
        implode("</option>\n\t<option>", $countries)
    );
}

Advise:

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell.
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.

The way I see it, your whole process can be boiled down to this:

<label for="country">** Select Country: </label>
<?php
$countries = file('countries.txt', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!$countries) {
    echo '<input type="text" name="country" id="country" maxlength="40" size="40">';
} else {
    array_unshift($countries, '');
    printf(
        '<select name="country" id="country"><option>' . "\n\t%s</option>\n</select>",
        implode("</option>\n\t<option>", $countries)
    );
}

Advise:

  1. Trimming your data should be done at write time (once), so that you never again need to trim your data ever again.
  2. file() will split your file's data line-by-line -- you can use flags to prevent newlines and empty lines (but those empty lines shouldn't be in your file to begin with).
  3. Move all styling to an external style sheet or at the very least out of your html markup -- it just makes your code harder to read with inline styling.
  4. Typically, <label> tags include a for attribute to connect with a field's id attribute. This is your choice to make.
  5. Don't repeat the same label. More generally, don't repeat yourself.
  6. I recommend not declaring single-use variables. If you are concatenating just to print anyhow, then just print as you go.
  7. I think you will have a very hard time finding goto in any modern professional applications. I recommend that you skip learning about this technique because when used, it gives a code an outdated smell. Other similar opinions: What is so bad with goto when it's used for these obvious and relevant cases?, Is GOTO in PHP evil?, Is GOTO a good practice?
  8. maxlength="40" size="40" is of no use in a <select> -- just omit these attributes.
  9. There is no benefit in repeating an <option>'s text value as its value attribute -- just omit that markup bloat.
  10. Because you just need to feed your country names to option tags, you can just implode() instead of using a foreach(). It's up to you if you prefer this technique.
  11. I am using array_unshift() to prepend an empty value to the array, this will provide the first empty <option>.
  12. Try to avoid relying on <br> for creating vertical space within your HTML document. Using fewer and better chosen html elements to craft your document will make it cleaner and easier to manage through styling.
Source Link
mickmackusa
  • 8.7k
  • 1
  • 16
  • 31
Loading