requinix

Sure, you can do that... if you write the code for it.

Matter of style. My opinion: If you set variables inside a condition (like an if or while) then the variable is only good inside the subsequent code block. Picked up that practice with languages that have stricter scope rules (like C). If you need the variable later then initialize it before the condition. if ($var = get_var()) { // use $var } // pretend $var doesn't exist $var = get_var(); if ($var) { // use $var } // can still use $var Other thing: allowing yourself that syntax increases the odds that you'll accidentally use a = when you want a ==. You'd have to pay closer attention to the code to decide whether that typo is intentional.

At only three fields it's not really worth trying to rework it into something that looks simpler but is actually more complicated. However you could use $_REQUEST as it contains both $_GET and $_POST values. And there's a bug in there: pretty sure one of the four ||s should be a &&.

Generally no, it's not worth it.

Default values for each? public function __construct($server = 'ftpofdoom.org', $port = 10021, $username = 'user', $password = 'pass') { If you need some kind of logic (like getting the default values from someplace) then yes, you would need something like that code. public function __construct($server = null, $port = null, $username = null, $password = null) { if (empty($server)) $server = // default value if (empty($port)) $port = // default value if (empty($username)) $username = // default value if (empty($password)) $password = // default value } You should actually do something even more complicated than that. Like if I give a $server but no $port then really the port should be the default 21, meanwhile no server and no port means pull both values from somewhere.

Just like with every. other. place. in PHP, you can't have more than one function with the same name. Besides, those two constructors have the exact same functionality. Just remove the first one and you won't even know it's gone.

It's not so much bad as it is incorrect. utf8_decode() actually decodes the text, but what you want to do is check that it was valid UTF-8. if (mb_detect_encoding($_POST['name'] . " ", "UTF-8")) { // valid UTF-8 The added space is because the string could end in the middle of a byte sequence and mb_detect_encoding() allows that (but you don't). Also, 1. Don't htmlentities() stuff when you put it into the database. Only when you output it. 2. Those str_replace()s should not be necessary. If you think they are then you should probably be using a function like urlencode() instead. But again, when you output stuff and not when you put it in the database. 3. strip_tags() if only if you want to completely remove HTML tags. If you don't care if there's HTML then don't use it. 4. If you're using mysqli then you should be using prepared statements, not building query strings by hand.

Assuming you don't want directory indexes anywhere, in your root .htaccess Options -Indexes That will show 403 errors.

#1 can be easily solved with sticking the functions in a file in a well-known place, but #2 is actually one of the few decent reasons. Use static methods, not instance methods. Make the class a mere container of functions, not an object of sorts. class MyHelpers { public static function makeArray($array) { return explode(',', $array); } public static function outputString($str) { return $str; } }

Anything wrong with just using simple functions?

What does the manual page say?

You don't have to store the number of votes (yay, nay, or total) anywhere. If you want to know then you can query your VOTES table.

Putting aside the fact that you could simply use the default values for those last two arguments, It's one statement. How much shorter do you want?

Are you familiar with hidden form fields?

[A-Za-z_]+ You require that the name have only letters and underscores. That isn't what you want. Change it so that it allows what you want it to allow.

You're too restrictive with the name. IMO allow pretty much everything and let your code find out if the name is correct or not.

If you like rolling your own SQL injection prevention and type conversions then sure. If you won't be executing one query repeatedly using different values, or don't care about inefficiency in doing so, then sure. If you don't care that the mysql extension is being deprecated then sure. If you don't want any of the advantages of prepared statements then sure.

Okay, well, the normal mod_rewrite stuff does that by default so... have you tried anything yet?

SELECT RIGHT(field, 4) FROM table WHERE field REGEXP "^[A-Z]{3}[0-9]{4}$" ORDER BY RIGHT(field, 4) DESC

Multiple simultaneous INSERT queries will be queued up, not executed in parallel. I believe all engines will write-lock tables during INSERTs, but won't necessarily read-lock. Thus if you run one INSERT with multiple values, all the values will be inserted in one (pseudo-)atomic operation. In other words a mysql_insert_id() - mysql_affected_rows() should be safe.

The first generators? Even though $xml->ROW is multiple elements, if you treat it like it was just one then SimpleXML will give you back only the first one. $xml->ROW->GENERATORS

Wrong. split() and explode() are similar but not compatible. Wrong. I'm even not sure what you're trying to say but it sounds wrong. And apparently you didn't notice that this thread is three years old. There isn't anything left to say beyond what's already been said so there really isn't any reason to bump it.

Don't use regular expressions when there are (much) better tools available. Like SimpleXML. $xml = new SimpleXMLElement('http://www.tva.gov/lakes/xml/OHH_F.xml', 0, true); foreach ($xml->ROW as $row) { printf("Release date: %s\n", $row->RELEASEDATE); printf("Time period: %s\n", $row->TIMEPERIOD); printf("Generators: %u\n", $row->GENERATORS); // or is this a bool? }

return only returns from the current function. It doesn't propagate up the call stack or something. With your checkInput() as it is now, the isValid* functions can return whatever they want and it won't do anything because checkInput() doesn't look at their return values. So make it look at their return values: if any of them return false then checkInput() should return false too.

This topic has been moved to mod_rewrite. http://www.phpfreaks.com/forums/index.php?topic=360875.0 What? I can't use the normal message sometimes? I have to change it? Come on, gimme a break.