3
\$\begingroup\$

The code below will generate an HTML select tag and options using a text file having list of countries. If the file doesn't exist or contains only empty / white space lines then generate an input box instead of select tag.


generate_select_tag_and option_from_list_of_countries.php

<?php

    $file = "countries.txt";
    $file_handle = @fopen($file, "r");
    $line = "";
    $valid_line_found = false;

    $label_input_html = '<label style="position:absolute;right:70%;color:black;">** Select Country: </label>' . "\n";
    $label_input_html = $label_input_html . '<input style="position:absolute;left:31%;" type="text" name="country" id="country" maxlength="40" size="40"><br><br>' . "\n";
    $label_input_html = $label_input_html . "\n";

    if ($file_handle == false) {
        echo $label_input_html;
        goto END;
    }

    while(!feof($file_handle)) {

        $line = fgets($file_handle);
        $line = trim($line);
        if (empty($line)) {
            continue;
        } else {
            $valid_line_found = true;
            break;
        }

    } // end of while loop

    if ($valid_line_found == false) {
        echo $label_input_html;
        fclose($file_handle);
        goto END;
    }

    echo '<label for="country" style="position:absolute;right:70%;color:black;">** Select Country: </label>' . "\n";
    echo '<select name="country" id="country" size="1" style="position:absolute;left:31%;" maxlength="40" size="40">' . "\n";
    echo "\t". '<option value=""></option>' . "\n";
    echo "\t". '<option value="' . $line . '">' . $line . '</option>' . "\n";

    while(!feof($file_handle)) {

        $line = fgets($file_handle);
        $line = trim($line);
        if (empty($line)) {
            continue;
        }
        echo "\t". '<option value="' . $line . '">' . $line . '</option>' . "\n";

    } // end of while loop

    echo '</select><br><br>' . "\n";
    echo "\n";

    fclose($file_handle);

    END:

?>

The file having list of countries is below:


countries.txt


Afghanistan
Albania
Algeria
Andorra
Angola
Antigua and Barbuda
Argentina
Armenia
Australia
Austria
Azerbaijan
Bahamas
Bahrain
Bangladesh
Barbados
Belarus
Belgium
Belize
Benin
Bhutan
Bolivia
Bosnia and Herzegovina
Botswana
Brazil
Brunei
Bulgaria
Burkina Faso
Burundi
Cambodia
Cameroon
Canada
Cape Verde
Central African Republic
Chad
Chile
China
Colombia
Comoros
Congo
Costa Rica
Croatia
Cuba
Cyprus
Czech Republic
DR Congo
Denmark
Djibouti
Dominica
Dominican Republic
East Timor
Ecuador
Egypt
El Salvador
Equatorial Guinea
Eritrea
Estonia
Eswatini
Ethiopia
Fiji
Finland
France
Gabon
Gambia
Georgia
Germany
Ghana
Greece
Grenada
Guatemala
Guinea
Guinea-Bissau
Guyana
Haiti
Honduras
Hungary
Iceland
India
Indonesia
Iran
Iraq
Ireland
Israel
Italy
Ivory Coast
Jamaica
Japan
Jordan
Kazakhstan
Kenya
Kiribati
Kuwait
Kyrgyzstan
Laos
Latvia
Lebanon
Lesotho
Liberia
Libya
Liechtenstein
Lithuania
Luxembourg
Madagascar
Malawi
Malaysia
Maldives
Mali
Malta
Marshall Islands
Mauritania
Mauritius
Mexico
Micronesia
Moldova
Monaco
Mongolia
Montenegro
Morocco
Mozambique
Myanmar
Namibia
Nauru
Nepal
Netherlands
New Zealand
Nicaragua
Niger
Nigeria
North Korea
North Macedonia
Norway
Oman
Pakistan
Palau
Palestine
Panama
Papua New Guinea
Paraguay
Peru
Philippines
Poland
Portugal
Qatar
Romania
Russia
Rwanda
Saint Kitts and Nevis
Saint Lucia
Saint Vincent and the Grenadines
Samoa
San Marino
Saudi Arabia
Senegal
Serbia
Seychelles
Sierra Leone
Singapore
Slovakia
Slovenia
Solomon Islands
Somalia
South Africa
South Korea
South Sudan
Spain
Sri Lanka
Sudan
Suriname
Sweden
Switzerland
Syria
São Tomé and Príncipe
Tajikistan
Tanzania
Thailand
Togo
Tonga
Trinidad and Tobago
Tunisia
Turkey
Turkmenistan
Tuvalu
Uganda
Ukraine
United Arab Emirates
United Kingdom
United States
Uruguay
Uzbekistan
Vanuatu
Vatican City
Venezuela
Vietnam
Yemen
Zambia
Zimbabwe

\$\endgroup\$
3
\$\begingroup\$

Consider using concatenation:

  • rather than $label_input_html = $label_input_html . ...
  • use $label_input_html .= ...

You're outputting HTML, so end $label_input_html with <br>, not \n.

Also see the "\t", so if are you doing all that for display purposes, like to show your work, that's fine.

I don't understand the END:. AFAIA that's completely unnecessary.

For an optimization, I'd put it all in one loop and test as you go for if you've found a valid line and if you're putting options or select. Additionally, rather than opening and reading line by line:

$lines = file($filename, FILE_IGNORE_NEW_LINES);

.. will return an array or FALSE.

Then you can loop through the array same way as the file, but without keeping a file open.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ The END: is a label used with the goto operator... not really used often in PHP but since PHP is a c-based language it supports it \$\endgroup\$ Nov 29 '21 at 21:11
  • \$\begingroup\$ Kinda thought that might be it, but at the same time, hoping not, lol. ("Who put goto's in my PHP?!?") \$\endgroup\$
    – bagsmode
    Nov 30 '21 at 22:56
2
\$\begingroup\$

Unless it is preferable to use fopen(), fgets(), fclose(), etc. - e.g. for the sake of performance, it could be simpler to use file_get_contents() to read all of the lines into memory and loop over the lines.


It is extremely rare to see goto within PHP code. Idiomatic PHP code would simply use conditional blocks. Even the PHP documentation for goto contains the XKCD about the keyword, also mentioned in this SO answer about when that keyword should be used.

1


Some comparisons use loose-equality operators - e.g.

if ($file_handle == false) {

It is a good habit to use strict equality operators whenever possible - especially given the values that are juggled to be equal to false.


At the end of the PHP file there is a closing PHP tag followed by another blank line

?>

Per the PHP documentation:

If a file contains only PHP code, it is preferable to omit the PHP closing tag at the end of the file. This prevents accidental whitespace or new lines being added after the PHP closing tag, which may cause unwanted effects because PHP will start output buffering when there is no intention from the programmer to send any output at that point in the script. 1

\$\endgroup\$
2
\$\begingroup\$

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.

  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.

\$\endgroup\$
0
\$\begingroup\$
<?php
// It might be good to write functions to describe at least what is happening by their name

    $file = "countries.txt"; // fopen is only safe when using this constant, don't accept user input as an argument
    $file_handle = @fopen($file, "r"); // don't use @ to suppres errors, it will make debugging so much harder https://stackoverflow.com/questions/136899/suppress-error-with-operator-in-php 
    // file_get_contents() might be better for your situation
    $line = "";
    $valid_line_found = false;

    $label_input_html = '<label style="position:absolute;right:70%;color:black;">** Select Country: </label>' . "\n"; // inline css can be improved to be written in it's own section / file, then you can reuse the styling.
    $label_input_html = $label_input_html . '<input style="position:absolute;left:31%;" type="text" name="country" id="country" maxlength="40" size="40"><br><br>' . "\n"; // also it's prettier to just generate html outside php tags and not in a variable, even if it's just for syntax highlighting
    $label_input_html = $label_input_html . "\n";

    if ($file_handle == false) { // comparison should always be done with === unless you have a good reason not to
        echo $label_input_html;
        goto END; // please don't use GOTO https://xkcd.com/292/
    }

    while(!feof($file_handle)) { // if you use file_get_contents() you can just loop with a foreach instead

        $line = fgets($file_handle);
        $line = trim($line);
        if (empty($line)) {
            continue;
        } else {
            $valid_line_found = true;
            break;
        }

    } // end of while loop
    // no need to comment end of loops normally

    if ($valid_line_found == false) {
        echo $label_input_html;
        fclose($file_handle);
        goto END;
    }

    echo '<label for="country" style="position:absolute;right:70%;color:black;">** Select Country: </label>' . "\n";
    echo '<select name="country" id="country" size="1" style="position:absolute;left:31%;" maxlength="40" size="40">' . "\n";
    echo "\t". '<option value=""></option>' . "\n";
    echo "\t". '<option value="' . $line . '">' . $line . '</option>' . "\n";

    while(!feof($file_handle)) { // you write a second loop but is not needed, you could combine them

        $line = fgets($file_handle);
        $line = trim($line);
        if (empty($line)) {
            continue;
        }
        echo "\t". '<option value="' . $line . '">' . $line . '</option>' . "\n";

    } // end of while loop

    echo '</select><br><br>' . "\n";
    echo "\n";

    fclose($file_handle);

    END:

?> // don't close php only files to prevent whitespace being send
```
\$\endgroup\$
2
  • \$\begingroup\$ It's generally better to write your observations as prose, rather than hiding them in comments of what appears to be a code-only answer. \$\endgroup\$ Nov 30 '21 at 11:39
  • \$\begingroup\$ @TobySpeight Good advice! \$\endgroup\$
    – Thomas
    Nov 30 '21 at 12:02

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.