KevinM1

Singletons are just another global variable. Globals are bad, as they break scope and encapsulation, both of which are cornerstones of good programming in general, and OOP specifically. Usually, if you're thinking about using a Singleton, it's a sign that you haven't thought about your design properly. Not always, but usually. A lot of "Should I use a Singleton here?" questions can be solved much more eloquently by using Dependency Injection/Inversion of Control (which are two names for the same thing). Look into the Symfony Dependency Injection Container (Google it) for a general idea of what it is.

WordPress is a piece of crap under the hood. Well written PHP never has the 'global' keyword in it.

The first thing that jumped out to me is that the buttons for both your deposit and withdrawal forms are merely icons (and confusing ones at that, as they look like tickets), which is the antithesis of accessible design. You need to have a description of what each section (deposits, withdrawals, etc.) means, in clear, concise terms, and you need to both ensure the form buttons look like buttons, and that they have text describing their use (a simple "Submit" would suffice). You can't assume anything when it comes to users with potential cognitive issues. More on accessibility: http://www.w3.org/WAI/intro/wcag.php

If you're using PHP 5+, you don't need the '&' in front of getInstance(). Aside from that, your class is a bare bones, textbook example/copy of a Singleton. There's nothing to critique or say, aside from the general warning about using Singletons.

No one has helped you because we're not entirely sure what you're trying to do. The code snippet you've provided is essentially gibberish. Suffice it to say that hidden form values won't be read until they're submitted to your script. Merely include-ing them won't work as your if-conditionals won't 'see' them. Ultimately, given how fundamental form handling and includes are, your best bet is to brush up on PHP basics. I'm getting the distinct feeling you're trying to run before you can even crawl, and that's the path of disappointment and failure.

Putting it in the JavaScript section for now, but it may be more apt to put it in Miscellaneous. This topic has been moved to JavaScript Help. http://www.phpfreaks.com/forums/index.php?topic=346732.0

Also, in your get_js_page_file() method, try: function get_js_page_file() { if (file_exists("./assets/js/pages/{$this->page}.js")) { $javascript = "<script type=\"text/javascript\" src=\"./js/pages/{$this->page}.js\"></script>"; } return $javascript; } Keep in mind, that if the file doesn't exist, your method will still attempt to return a then null $javascript variable since the return statement is outside of your conditional.

And what's the error on line 21...?

What are your errors? Where, specifically (read: line numbers), are they appearing? How are you trying to use your class?

1. This is PHP Freaks. We have a PHP focus, with diminishing focus on various relational databases, JavaScript, HTML, and CSS since they're all used in conjunction with PHP. Generalized boards, such as our "Other Programming Languages" board, are a courtesy. While you're correct that mathematics is applicable to all programming languages (and, indeed, is the genesis of those languages), since this is a PHP-first forum, that's the lens through which the vast majority of our members view things. More to the point, that board's focus is on solving mathematical problems with PHP. Not Java, not Python, not Perl, not C#, etc. 2. Coming out and blatantly calling something stupid is a surefire way to not get the staff to take your (or anyone else's) suggestions seriously. Learn some tact and try again.

It's pretty nice. On the index, I would move the SiteLock image up a bit so it's inline with the Sign Up button and Facebook button. You may want to see how putting the "We Currently Offer..." and "Coming Soon..." groups side-by-side rather than being centered atop one another. There's just a lot of empty space on either side, so it makes the site look a bit top heavy. I'm a bit torn on the footer navigation. I like that the look mirrors the header navigation, but since those links are clearly not top-level in terms of importance, giving them such treatment is confusing. I keep wondering if some of them should be brought up top (like, say, the FAQ link). It's something else to play with. Certainly not bad the way it is, but it's triggering my Spidey Sense. Page-By-Page - Quick Tour: Bring the "Everything you need to know about MusicMuse:" image up a bit. You should probably bring the shadow above it up a bit as well. Learning: The term "Cores" in the side navigation is a bit confusing as "Fundamentals" is a synonymous term. Also, "Multiple camera angles" of what? Video? Still pictures (since you mention printable music in the item above)? You should go back and tighten up the language used on this page, as you're making assumptions that the user knows what you're referring to. Teachers: Bring the teacher listing up a bit (similar to what I suggested on Quick Tour). Make each row highlight on mouseover, and make each instructor's name a link to their profile as well. The rest is spot on. Hell, I may use your forms as examples to other users on how to make them look attractive. Overall, nice job. Try playing with some tweaks, but there's nothing fundamental that needs to be addressed.

You don't need to connect to a database twice. Once you're connected, the connection stays open until the script ends, you manually close it, or you attempt to open another connection and you don't specify to keep the current one open. Similarly, you don't need a while-loop if you're only retrieving a single row of data. The entire point of loops is to iterate over multiple items. You also neglect to put brackets in the loop anyway. Ultimately, you need to do some bare bones basic debugging. That means turning on error reporting by putting the following at the top of your files: ini_set('display_errors', 1); error_reporting(E_ALL | E_STRICT); which will display all errors, warnings, and notices when things break. That will help you narrow down your problems. You also need to escape your incoming string (read: text) data. Stripping tags does nothing to protect your database from injection attacks. mysql_real_escape_string - learn it, use it, love it.

Here's the thing: you shouldn't be building queries like this in your animals anyway. The animals should be generated by another source, like an animal repository object, that would handle the business of mapping the animals to the db and vice-versa. class AnimalRepository { private $db; // <--- this is the database itself, likely a MySQLi or, even better, PDO object public function __construct($db) // <--- best to use Dependency Injection to wire up the db to the repo automatically { $this->db = $db; } // methods for retrieving, saving, etc. animals goes here } abstract class Animal { // methods that ALL animals will have } class Bear extends Animal { private $subspecies; public function __construct($subspecies) { $this->subspecies = $subspecies; } // Bear specific methods go here } /* Note no PolarBear class. Unless PolarBears have unique behavior that's different than other bears (and, really, they don't), there's no reason to subclass. */

This topic has been moved to Miscellaneous. http://www.phpfreaks.com/forums/index.php?topic=346275.0

This topic has been moved to HTML Help. http://www.phpfreaks.com/forums/index.php?topic=346255.0

You shouldn't have function polarbar at all. Remove it in its entirety.

This: function polarbar() { // Note: the next line will issue a warning if E_STRICT is enabled. Bear::bear(); } Is completely unnecessary. You're already calling Bear's constructor in PolarBear's __construct() method. What are you trying to do here? As for the rest, your methods are not static, thus trying to invoke them with :: won't work. Static methods are executed in class scope. Calling Bear::eat($units) makes no sense because there's no object in play, so there's no $this variable to work with. Read more: static Now, to be absolutely clear, I'm NOT suggesting you turn your methods static. Since static methods can be invoked anywhere simply by writing Class::method(/* args */), they are therefore global. Globals are the antithesis of good OOP.

You need to pass the appropriate values into your functions via their argument lists. In other words: function launch($skillLevel) { // rest of function } // AND, when you want to invoke it: launch($skillLevel); Also, NEVER use 'global'. Pass parameters through a function's argument list. That's why it's there. In fact, stop using whatever source you're using to learn PHP. 'Global' is a clear sign of doing it wrong, which calls into doubt everything else your resource is trying to instruct. Regarding functions (and just about everything else), go to the manual for more information: functions

Split your categories from the individual items that belong to them: categories: cat_id | cat_name items: item_id | cat_id | name | description | date Then, assuming that members can have multiple items, make the following pivot table - items_members: item_id | mem_id That table is the magic glue that makes your many-to-many relationship between items and members work. You don't need a native id column in that table as each row is unique. Example: item_id | mem_id 1 14 1 25 1 03 1 66 2 14 2 15 3 66 4 15 4 03 4 25 In the example above, member 15 owns/is related to items 2 and 4. Similarly, item 1 is owned by members 14, 25, 3, and 66.

What? No, not at all. Relational databases are supposed to have as many tables as needed to avoid insert/delete anomalies. This, in turn, is why tables should be normalized. Cramming a bunch of info into columns like a spreadsheet is exactly the wrong way to go, and negates the entire point of using a relational database. @floridaflatlander, the other sources you looked at had the right of it. Repeated values in columns is often a sign that your database design is bad. Can you show how all of your tables are structured currently? Getting your database design right at the start will protect you from headaches down the road.

I'm a bit confused on what you want us to critique if you didn't do any of the major work on the site.

I will. I just made my first OOP class and I would really like to learn more about it, because I feel that it is the next step for me as a coder. Just because your using classes does not mean your coding in OOP. It's a first step, but it's not the entire picture. Hmm Interesting. Care to explain? Stuffing a bunch of thematically similar functions in a class isn't OOP. If it were that simple, then why would dozens of thick books exist on the subject? OOP is a completely different design methodology than procedural programming. It revolves around encapsulating both structures and behaviors, and dynamically using them/plugging them in/swapping them at run time.

There's a difference between creating utility methods, like you're doing here, and using PHP's built-in __get and __set magic methods (like I did in that Stack Overflow link I gave you earlier) to remove the tedious boilerplate of writing simple/dumb setters and getters altogether. It's a fine distinction, but an important one. Right now, you have general methods that, if accessed directly on the object, would look like: $report->getProperty("employeecount"); $report->setProperty("familylogins", 44); Which I'm pretty sure is not what you're after. Compare that with using __get and __set: echo $report->employeecount; $report->familylogins = 44; Keep in mind that __get and __set are only invoked if no property of that name actually exists within the class. That's why I had the props array and no other fields in my Stack Overflow example. Also keep in mind that my Stack Overflow example quickly falls apart if different setters or getters need to execute specialized code in order to do their jobs, like, say, access a db. At that point, __get/__set would likely be a long switch that delegated to specialized private methods, which, really, would negate much of the point of using them. Of course, all of this begs the question - do any of the Report's fields need to be directly accessed? Don't use setters and getters because that's how everyone does it. Ask yourself: "Do I need to be able to set/get this particular field independently from the others?" Only code the functionality you'll actually use.

Well, there are a couple of properties you can get rid of. Anything that's a total of other fields can simply be generated when you access them. So, if you have a method named getTotalLogins(), you don't need a $totalLogins property. Simply compute the total logins in the method. That said, I'm a bit confused about where the db is coming from. You have your set/getProperty methods as public, which makes no sense. These are utility methods which should not be exposed to the public. Rather, your named setters and getters should be public, and the utility methods private or protected. Ultimately, I can't help but think you're approaching it from the wrong end. An Employer should generate a report, not the other way around. I mean think about it - when someone passes an id to your Report constructor, what context will they assume they're in? They'll think that the id is the Report id, not the Employer id. And doesn't it make more sense to do something like: $employer = EmployerFactory::getInstance($id); // Look up Factory Method on Google, or in Zandstra's book, which you should buy $employer->generateReport(/* optional args for report type, or date, or something */); // send/display report (which could be rolled into generateReport) ?? As for the rest, I like ignace's approach. As to how his EmployeeCollection fits with an Employer and a Report, an Employer would contain an EmployeeCollection. From that collection, the Report would be generated.