KevinM1

Ah, didn't mean for it to come across as harsh. Just wanted to give you (and anyone else reading this) a resource for color picking. I tend to suck at this stuff myself. I'm no artist, and my idea of a good UI is something spartan and functional. Colors are usually a last minute concern for me.

You did explain to them that PHP 6 won't be coming out, right? The old list of improvements will simply be rolled into PHP 5. AFAIK, the two biggest - namespaces and late static binding - have already been integrated.

A site to give you some inspiration/hints about colors: http://www.colourlovers.com/ (look at their palettes)

Not a book, but a decent article on writing good JavaScript: http://net.tutsplus.com/tutorials/javascript-ajax/the-essentials-of-writing-high-quality-javascript/

Looking over your code, I think you need to take a step back and think about your design. There's some repeated code here. Also, the fact that you place a function definition within a conditional shows a lot of confusion. Good code reads well. A lot of this is gibberish. Take a look at your function - it does too much. You're trying to both fix a filename and do some database work with it. You're also assuming that the function will automatically know the context in which it was invoked, which is simply not true. I've done a bit to help your overall structure. More can, and should, be done. Also, read my comments. Finally, do your best to do things right. I get the feeling you're trying to rush, and you're learning bad habits because of it. Structure, clarity, no repeated code... these are what you should strive for. Getting proper output does not mean you're writing good code. <?php include "../php/connect.php"; /* cleanFilename * @params: * $str - string, filename to be cleaned * $username - string, username for the avatar * * @return value: * string, cleaned filename */ // function tries to do too much. It shouldn't both clean a file name AND set an avatar function cleanFileName($str, $username) { $cleaner = array(); $cleaner[] = array('expression'=>"/[àáäãâª]/",'replace'=>"a"); $cleaner[] = array('expression'=>"/[èéêë]/",'replace'=>"e"); $cleaner[] = array('expression'=>"/[ìíîï]/",'replace'=>"i"); $cleaner[] = array('expression'=>"/[òóõôö]/",'replace'=>"o"); $cleaner[] = array('expression'=>"/[ùúûü]/",'replace'=>"u"); $cleaner[] = array('expression'=>"/[ñ]/",'replace'=>"n"); $cleaner[] = array('expression'=>"/[ç]/",'replace'=>"c"); $cleaner[] = array('expression'=>"/[ß]/",'replace'=>"ss"); $ext_point = strpos($str,"."); if ($ext_point===false) return false; $ext = substr($str,$ext_point,strlen($str)); $str = substr($str,0,$ext_point); foreach($cleaner as $cv) { $str = preg_replace($cv["expression"], $cv["replace"],$str); } //Require approval from flash and handling from flash for files $fix = str_replace(" ","_",$str).$ext; $setavatar = mysql_query("UPDATE accounts SET avatar = 'http://wiistream.net/users/$username/images/$fix' WHERE username = '$username'"); return $fix; } if(!empty($_FILES)) { //sanitize filename $_FILES['Filedata']['supplied_name'] = $_FILES['Filedata']['name']; $_FILES['Filedata']['name'] = cleanFileName(utf8_decode($_FILES['Filedata']['name']), $username); // $username doesn't exist yet if($fp = fopen("log.txt","a+")) { $contents = fwrite($fp, "===================================================\r\n"); $contents = fwrite($fp, "$ _FILES['filedata']: \r\n"); $contents = fwrite($fp, "FILES: ".print_r($_FILES['Filedata'],true)); $contents = fwrite($fp, "$ _GET: \r\n"); $contents = fwrite($fp, "PROPERTIES: ".print_r($_GET,true)); fclose($fp); } if(isset($_GET['d'])) // note how I removed the '@' - don't squelch errors. Test for them, and write your code appropriately { $_SESSION['movedfileprops'] = array(); $_SESSION['movedfileprops']['type'] = $_GET['type']; $_SESSION['movedfileprops']['name'] = $_GET['name']; $_SESSION['movedfileprops']['size'] = $_GET['size']; $_SESSION['movedfileprops']['dir'] = $_GET['dir']; $_SESSION['movedfileprops']['a'] = $_GET['a']; $_SESSION['movedfileprops']['ft'] = explode(",",$_GET['ft']); $_SESSION['movedfileprops']['allowed'] = (in_array(LTRIM(strtolower($_SESSION['movedfileprops']['type']),"."), $_SESSION['movedfileprops']['ft']) ? "yes" : ($_GET['a'] == true ? "yes" : "no")); if($fp = fopen("log.txt","a+")) { $contents = fwrite($fp, "$ _SESSION['movedfileprops']: \r\n"); $contents = fwrite($fp, "SESSION: ".print_r($_SESSION['movedfileprops'],true)); fclose($fp); } if($_SESSION['movedfileprops']['allowed'] == "yes") { $filepath = getcwd()."/".$_GET['dir']."/"; if(!file_exists($filepath)) mkdir($filepath,0777); chmod($filepath,0777); if(move_uploaded_file($_FILES['Filedata']['tmp_name'], $filepath . $_FILES['Filedata']['name'])) chmod($filepath.$_FILES['Filedata']['name'], 0777); } if($fp = fopen("log.txt","a+")) // repeated conditional. Consolidate your code { if(file_exists($filepath . $_FILES['Filedata']['name'])){ $contents = fwrite($fp, "Upload Success: ".$filepath.$_FILES['Filedata']['name']." \r\n"); } else { $contents = fwrite($fp, "Could not upload to: ".$filepath.$_FILES['Filedata']['name']." and I don't know why."); } fclose($fp); } } } $username = $_POST['username']; $username = stripslashes($username); $username = mysql_real_escape_string($username); $sql = mysql_query("SELECT avatar FROM accounts WHERE username = '$username'"); $rows = mysql_num_rows($sql); if($rows > 0) { // why were you trying to write your function definition here? echo "&msgText=$username!\n"; } ?>

Moreover, parentheses enforce associativity. If the order of operations of an expression aren't known, extra parentheses can be used to explicitly enforce a particular ordering. http://www.php.net/manual/en/language.operators.precedence.php

Do everyone a favor and put your code within CODE tags. It's a mess to read without proper formatting and syntax highlighting.

It's a scope issue. Your function cannot see $username since it's defined outside of your function. You need to pass it into your function. You also seem to be having a problem understanding the difference between defining a function and invoking/executing a function. Your function definitions shouldn't be placed within control structures (if/else conditionals, loops, etc.). You need to define it separately from where you want to run it. In other words, your function code only defines how the function should work. It doesn't actually execute it. And, again, since $username is defined elsewhere, your function cannot 'see' it. NO. Do NOT use 'global' for this. It's a crutch that leads to bad design and major problems with code maintainability. Learn to write good code.

Just pass in a date string to a Date's constructor. If your date format is YYYY-MM-DD HH:MM:SS, replace the punctuation with commas (see the bottom of this page for an example: https://developer.mozilla.org/en/JavaScript/Reference/global_objects/date).

Have you: Run the query directly (e.g., within phpMyAdmin)? Tested to see if your mySQLi object is actually being constructed and populated correctly? Turned on all error reporting?

Simply put, your form processing logic is off. You need to see if errors have actually been encountered before attempting to insert field values. Right now, you really only see if an email address exists before merrily inserting data. In short, go through your script by hand. Take note of exactly what your conditionals are doing (hint: it's not what you want them to do). Let's say username is empty. You create an error message, but do you actually test to see if that error condition exists before going to the db? No. That's one example of many in your script. Also, don't use the '@' when outputting. Next to people using 'global', it's probably the most misused part of PHP.

Yup, it's possible. Lightbox2: http://www.huddletogether.com/projects/lightbox2/ I use it. Very simple to implement.

(Emphasis mine.) No you haven't. This. Even if the ads have been removed, the layout is still borked. Code blocks are broken. Also, 'testing' on the live site? Are you kidding me? There's a reason why things are tested before being implemented live. The only thing these new ads have done is make the people who actually run the place look incompetent, which is a great message to send people looking for help. I guess we shouldn't expect more for an owner who makes a mere handful of posts in a two year span. Actions speak louder than words, and the actions show someone who doesn't really give a crap about the community, despite semi-elegant sales language to the contrary.

Code blocks still break the layout, FWIW.

A -1 means the query returned an error.

You'd have to pass it through sessions.

Active Record is a design pattern. Many ORMs use the pattern to keep it simple on their end. A 1-1 mapping is easier to create than something that picks and chooses from various tables. See: http://en.wikipedia.org/wiki/Active_record_pattern I believe you can use an ORM on a database view, which would give you the best of both worlds. The ORM would map 1-1 with the view, which could be constructed out of data from multiple tables. Updates could also take place with the view, where the updated view passes the update to the db itself. This is possible with .NET and the Entity Framework, but I'm not sure about PHP ORM solutions and MySQL.

I wouldn't construct a User object at all unless their credentials are legit. That way, you save yourself the trouble of building an object which may not represent an actual user, and the mere presence of a User object tells you that the person is logged in. No need for a separate field. So, send the raw data to the Authenticator, have it do its thing, and if successful it returns a populated User. Regarding the use of a User object in updating a profile, in a lot of cases you wouldn't have to manually destroy the object. Either you'd keep it loaded in sessions if you need to keep it around for other processes, or it would be destructed automatically when the end user is taken to a different part of the site (update confirmation screen, or even a reload of the current page).

First, thanks for the sample code!! (I'll look at it tomorrow when my brain is fresh, and see if I can actually understand it?!) Second, since you brought it up, can you please explain what ORM is and why you recommend it? (I know it has something to do with how you lay out your data between the Object and Database Worlds, but how does it relate to things like ActiveRecord and why is one better for me than the other??) Thanks, TomTees ORM stands for Object-Relational Mapping (or, in this case, Mapper). It's a system that maps relational data (e.g., your db data) to objects in code. This saves one from having to manually write and execute CRUD functionality, as these objects will have CRUD implemented within them, which are invoked through their methods. It's much easier to do something like*: $person = ORM::factory('person', 3); // get the person with the id of 3 from the person table $person->name = "Forrest Gump"; $person->age = 43; $person->save(); Than to deal with the db directly. It's an abstraction layer. Also, many ORM systems use the Active Record pattern to do their thing, like the example above shows. *Code is a canned example of Kohana's built-in ORM.

Well, there's really no set answer to these kinds of questions because it depends on the context of your current situation. OOP is filled with "Don't do X, unless Y" kinds of situations. Accessors fall into that group. Not all objects need accessors. But some (data objects) rely on them to be useful. The key is to figure out who needs access to what in order to work. Here's another one for you: PHP lacks a struct-like data structure. In order to fudge it, a lot of developers create small, simple objects with public fields. Not a good habit to get into in a general sense, but not entirely horrible if you're only dealing with POD (plain old data). For example, Kohana's ORM does this because model objects are a 1-1 map of a db table row, so it streamlines assignment. The only meaningful activity connected to the object is saving its data back to the db. But, yeah, I hear you. It can be maddening in the beginning because a lot of it boils down to context or opinion.

Why would I need a User object in the first place (after creating a User in the database)? Well, what else could "load the User object into memory" mean? I'm only responding to what you wrote. You lost me... I was asking what purpose having a User object loaded into memory would be while you are just surfing. You suggested using sessions the normal way which means I wouldn't even need an object, right? So I'm just trying to figure the purpose of a User object. Well, like I tried to say last night, in most cases you wouldn't, unless you needed to act upon the user object itself. Take something like a user editing their profile. It makes sense to load an object with the current data stored in the db, have the system work with the object to make the changes, then have the object save the new changes back to the db. Hint - think static. Not following you. If user registration is a one-time-use activity, you don't need to go through the hassle of instantiating an object for it. You don't care about actually having an object in hand, but rather you just want access to its registration method. Make that method static and call it: RegistrationService::registerNewUser(/* data */); For the former, not necessarily. For the latter, I would. At the very least it's more convenient to deal with objects in that case than with the db directly, especially if you have a decent ORM. So just like anything left in memory - which is all objects are, right? - you do it to speed things up, right? Just like the concept of cache... Somewhat. It's more to make it faster on a human level. If I can deal with an object, I don't need to worry about the implementation details of how to record and store that data. It's hidden behind methods. I'd much rather deal with an object's interface than have to it the other way. Remember, OOP is a methodology to make programming easier for humans. If this was about memory efficiency, we'd all be writing binary or assembly. Again, it's often a matter of convenience. For one, you can keep the object in memory, giving you delayed execution if you want it. Second, you can represent CRUD as methods. Third, for me, it makes more sense conceptually to deal with objects. Since this is OOP, why wouldn't I represent a user as an object? No. Not, "Why would I represent a User as an object". I'm asking, "What justification do I have for keeping a User object tying up my memory?"" (Unless I'm doing something major like changing an e-mail, updating preferences, etc.) That I can see, but then that leads me to ask... 1.) Exactly how much memory would it take up to leave a User object up and running in memory? Right now, my fields would be... - email - password - firstName - middleInitial - lastName - displayName - agreeToTerms - age - lastVisited - loggedIn - userRole and maybe a half-a-dozen more fields?! I'd say your User object is a bit bloated. I wouldn't store password info in an object at all. You're most likely not going to use it. Leave it in the db, and retrieve it when necessary. AgreeToTerms is a useless field. Presumably there wouldn't be a user if they didn't agree to terms. The same goes for loggedIn - there wouldn't be an instantiated object if they weren't logged in. Think about trimming it down. You don't need a 1-1 mapping between your object and its backing db row. At the very least, as much as its component data structures. One byte per string character, 4-8 bytes per int, etc. There's some overhead for it simply being an object. Not sure how much methods take. Yeah, the bottleneck is at the db in most cases. Not sure. I have the feeling they use a simplified user object until you go to checkout, or another account area, where it then loads the full object. I don't think credit card info ever leaves its db. All that said, I wouldn't worry about memory at this time. The key is to get an understanding of the principles first. Performance should come later.

As you found out, that's exactly what I'm saying. Also, again, don't be paranoid about having things public. How else can objects talk to one another, let alone be instantiated? Of course fields should be either private or protected, but most of the methods you'll write will be public. A constructor is simply a method. And, yes, accessors do have their place as well. Hell, C#, a language that's much more object oriented than PHP, has a property mechanism just for that reason. A lot of objects need to know about other objects' current state in order to work. Look at just about any object that represents a database record. How are you going to get at its data without an accessor? You linked to an article about how accessors are bad, but did you ever bother to read the comments? They're pretty enlightening: http://www.javaworld.com/community/?q=node/1759#comment-32951 It's not scary. It's written very plainly. The biggest hurdle is that the examples are written in C++ and Smalltalk. Their syntax is similar enough to PHP's to make it a non-issue.

You're not planning on making all your classes static or Singletons, are you? Nope. Not at all. But based on what you said, it sounds like I can make all constructors "private"... TomTees Not in the way you're thinking. You can't make them private and expect normal behavior, which is why I specifically mentioned Singletons and static classes. Singletons use a static method to return an instance of itself. There can only be one instance of a particular Singleton in existence EVER. Singletons are also global in nature because they use a static method for creation/assignment. Best to stay away from them. Generic static classes shouldn't be instantiated at all. Instead, data is passed into a static method and a result comes back, often an object. I'd say the vast majority of classes you'll write will have public constructors. That's okay. Encapsulation is good, but there still needs to be a way to turn on an object and initialize it somewhere in the system. You should do yourself a favor and buy the Gang of Four's book. It will address all these fundamental questions and provide relevant examples.

You're not planning on making all your classes static or Singletons, are you?

Why would I need a User object in the first place (after creating a User in the database)? Well, what else could "load the User object into memory" mean? I'm only responding to what you wrote. Hint - think static. For the former, not necessarily. For the latter, I would. At the very least it's more convenient to deal with objects in that case than with the db directly, especially if you have a decent ORM. Again, it's often a matter of convenience. For one, you can keep the object in memory, giving you delayed execution if you want it. Second, you can represent CRUD as methods. Third, for me, it makes more sense conceptually to deal with objects. Since this is OOP, why wouldn't I represent a user as an object? Yeah, it's overkill if you only want to display their name on the screen, but for anything of reasonable complexity it makes sense to use an object.