- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Let me restate my opinion: you should try to make your code as resilient to misuse as possible, i.e. make it ROBUST; that makes 'accidents' (as your call them) that much less likely to happen, and easier to diagnose and fix. In this specific case, for instance, this:
$c = new Criteria(); $c->addCondition(new GreaterThanOrEqualToCondition('startDateTime', $this->getToday())); $c->addCondition(new EqualToCondition('available', 1)); $c->addOrder(new AscendingOrder('startDateTime'));is obviously MUCH better (and clearer) than this:
$c = new Criteria(); $c->leftParenthesis(); $c->field('startDateTime'); $c->greaterThanOrEqualTo(); $c->quote($this->getToday()); $c->rightParenthesis(); $c->andCriteria( Criteria::newCriteria()->field('available')-> equal()->quote(1) ); $c->addString(' order by startDateTime;');In the first snippet, by construction, it is impossible to have unbalanced parentheses; in the second one, it is up to the user of the API to make sure they didn't forget a call to rightParenthesis() or leftParenthesis(). So to some extent, the code in the first snippet is more idiot-proof than the second one. That doesn't mean that the code in the first snippet is perfect, but it is harder to do something wrong with it, and the price to pay for switching from design #2 to design #1 is minimal.
Sure, peer review and code testing would catch issues like unbalanced parentheses (or there is something wrong with your developmenet / QA cycle), but it is much better if the design of your SQL builder class prevents you from making that mistake in the first place.
To give you another example: if I create unit tests for my classes, I make it harder for someone to modify code in a way that violates unit tests (of course, it is not perfectly idiot-proof: an idiot could disable unit tests just to "make things work", but someone who does this is unprofessional and should steer clear of the codebase). If I set up my build system to refuse to even push a build to a QA stage if said build doesn't pass unit tests, not only have I idiot-proofed my build system, but I have also just eased QA's and other developers' work.
When I advocate idiot-proofing in general, I mean using techniques like creating unit tests (to ensure that whatever changes someone else may do to a piece of code will not violate the assumptions and guarantees that that code provides), making defensive copies, making classes immutable when possible (that makes thread-safety a breeze), making arguments constant unless there is a reason not to, enforcing constraints such as foreign keys in a database, designing fault-tolerant protocols, etc. I don't quite get how confidence that arises from robust code or from guarantees (such as "class Foo is immutable, so I can safely share it among several threads") promotes "a false sense of security" or would make me more careless.
In the case of UI design in particular, idiot-proofing is quintessential: you cannot and should not assume that the user will always know how to do things the right way, that he won't make mistakes, or that he won't have malicious intents. For instance, he starts filling out a wizard in your web application, then goes back to the home page, and your webapp starts behaving weirdly because "the user is not supposed to do that", your UI is crappy.
As with all good things however, there is a limit to how much effort you should expand to bullet-proof your system; it's really hard to guard against ANYTHING, and sometimes the performance penalty may be too high. But for refactoring / design that has little to no impact on performance and makes your code more resilient to bad input, I contend that there is no reason not to do it.
Admin
Reminds me to zend framework ;)
Admin
So, what's the WTF here? It's just the implementation of a search criteria class. Ok, it looks kind of ugly, but it's not really a "WTF?!".
Try looking at Hibernate or JPA if you're a Java programmer. They have incredible "WTF" APIs similar to this, so that you can express a simple SQL query into an incredible unreadable block of Java code.
Here's an example of how you can do a simple SQL query in an "enterprisey" way...
Admin
Funny - that's quite along the lines of what passed my mind when whitnessing your earlier fits. Meh. :)
Admin
Admin
TRWTF is Jason defending this trash by pointing out it's source as if it serves as some sort of validation of suitability. Get over yourself mate, there's no need for this ridiculous complexity, and a prior knowledge of the technology utilised is barely required to establish this.
I guess you're probably cut from the same cloth as the guys that implement this stuff in the first place.
L8R DooooD!!
Admin
Let me be clear, I don't have a problem with the concept of ORM*. I do have a problem with that Arrogant SOB thinking everyone - PHP programmers and non-PHP programmers alike - should be familiar with EVERY PHP extension, especially ones not on the standard extension list.
I DO have a problem with his implementation of ORM of choice - it's inefficient, violates OOP best practices (naming), etc.
==================================
If you're "filling in the blanks" in a stored-procedure-like query like "Select * From siteUsers Where userId=? and password=?" where you fill in the ?s then you have a place where as far as I'm concerned using ORM is Cargo Cult.
Using ORM in any place where a stored proceedure would be appropriate is an antipattern-like. The ORM is an inappropriate expense of memory and computation.
Not only would you have to spend memory and compute cycles giving commands to the ORM object, manipulating it's memory, then finally "rendering the query" but then the DB engine has to run it's query optimizer and resolve indexes. With a Stored Proceedure you have none of the ORM expense and the query optimizer is run at the time of SP creation not SP invocation.
Admin
This sounds a lot like "hey, I know, I'll solve this security problem using PKI!". Or "I'll solve this data-exchange problem using XML". Or "I'll solve our mail problems using Lotus Notes". Or ...
Admin
Admin
Who's talking about "complex solutions"? Those are your words, not mine, and yet somehow you use them to conclude that I am not the genius I am. That says more about your defective deductive skills than it could possibly say about me.
Admin
I tend to use classes adhering to the principle "if it's a list, pass an array". That gives you stuff like:
$db->getrow( $table, $array_of_fields, $array_of_where, $array_of_options );
Pretty readeable IMHO, and very well suited to dynamic query creation.
Admin
Really? I didn't work it out. I was looking at the code and thinking "no way, I don't buy it... this guy is just making this up.". Then I thought "nah, maybe there really are people that ... out there.". I don't even know how to describe that... Stupid? Wierd?
On the other hand, he's probably writing this from memory and not copy-pasting, so I guess the sql not matching the query doesn't have anything to say about the veracity of the story.
That's some really freaky shit man. I often have a really hard time telling people how good a programmer. I mean, I know I'm no Bjarne Stroustrup or Alexandrescu, but compared to a lot of the shit you see out there, I'm like Superman. I'm amazed at the level of incompetence that can be tolerated in our profession.
Admin
I completely agree. It has been suggested to me by other developers that i should convince my employers to use ORM for our solutions when in my opinion it would be silly to do so as it would only complicate that which is already simple. When i ask why i should even consider it i generally received responses that equated to 'because everyone else is...'.
Admin
I disagree; the whole point of ORMs is to simplify the mapping from result sets to objects; yes, you trade off some CPU cycles for ease and development time, and as with all decisions involving a trade off, you need to determine whether you're better off using it or not. It's not a silver bullet.
ORMs tend to be good when dealing with rich models, and poor for complicated queries such as reporting. Depending on the profile of your application, different flavors of ORMs may or may not be appropriate. But there are other uses of ORMs beyond "algorithmically generating query from user input".
As a bonus point, ORMs make it very easy to incorporate things such as a cache (not meaning that an application using an ORM should be decent performance-wise only if you use a cache).
Admin
Perhaps "fuck up" is too strong. But yes once you forget to put in a right paren, you've fucked up. Now you spent the next hour, or day trying to figure out where the right parent is missing in a huge query. There is NO reason to force the programmer to add a right paren via a method, so why force them? This isn't about idiot proofing, it is about making things easier to use.
You have already "over complicated" the system by adding leftparen and rightparent methods. Simplify the system, adding a group mechanism.
As far as "idiot proofing" goes, I'm not suggest idiot proofing anything. As we've already seen idiots can generate some pretty bad code, no matter what. I am just saying make it easier to program.
Admin
I don't know what SQL the above code sample would generate and frankly I don't care, because it's a lot easier and makes a lot more sense to write that SQL yourself, encapsulate it in a stored procedure, then call the stored procedure.
That way you have code that can easily be profiled and tested, that you can put under version control, that won't give compile errors if the underlying generator changes, etc...
If you're working with databases, learn SQL and how to best use it, instead of hiding behind flimsy frameworks that allow you to pretend that SQL doesn't exist. At best, code generators such as your QueryBuilder are a great solution for a problem that doesn't exist.
Admin
there are much much simplier ways to write abstractors between the underlying DB implementation and the code.
a foreach on mysql_fetch_assoc is pretty simple if you don't need abstraction.
While everyone thinks it's ok to waste compute cycles these days - I don't. Maybe it's because I have worked on coding games, maybe it's because I currently work on a system that processes more than 1000 credit/debit card transactions per second.
Either way I don't accept useless overcomplications, it's fundamentally anti-performance-computing.
Admin
Use Doctrine ORM for a little while. Then you'll realise that Propel is a WTF.
Admin
As I said, ORM is not a silver bullet. In some situations it is appropriate, in some it is not.
Now, in your credit card transaction processing system, I may be wrong, but my guess would be that your bottleneck is more likely to be the database I/O than CPU cycles; if you're using an ORM in that situation, I wouldn't worry about the overhead of the ORM (and CPU cycles for that) as much as the SQL generated by it (which, if improperly done, could result in abysmal performance - think N+1 select problem).
The solution you propose for MySQL is too simplistic - it just doesn't accomodate relationships that well; if I have a query that retrieves a "user inner join his billing details", translating that into an object graph takes a little more than a simple foreach loop. That IS the whole point of the ORM: take care of the mapping from result sets to objects for you.
Contrary to what the poster before you said, using an ORM as "let's forget about SQL" is NOT the right way to use an ORM. The right way to do it is to configure you ORM, monitor the SQL being generated, and fix your model / your queries as appropriate. If you see that the ORM performs 30 queries to retrieve your billing details (instead of one query with joins), then the problem is in YOUR code / mapping, not in the ORM, and you'd better fix it if you want your application to perform well performance-wise.
Frankly, if used properly, the only overhead of an ORM is that it has to have a generic concept of how to map result sets to objects, whereas custom code that you could write could do it for you with specific knowledge about your tables and columns; and yes, your custom code would probably be performing slightly better than a generic ORM (because it doesn't need to use reflection / bytecode instrumentation), but is it worth it compared to the time it will take to do it? Not always.
Admin
(STATE_ID NUMBER NOT NULL PRIMARY_KEY, STATE_CD VARCHAR2(2) NOT NULL, STATE_NM VARCHAR2(50) NOT NULL)
It does require quite a few more joins, since all other tables have a STATE_ID foriegn key, but if California or New York ever change their names, we will be sitting pretty!
Admin
This isn't a Daily WTF. This is an OMFG!
Admin
Looks like someone's glass house just got shattered. Or did I mean 'sacred cow just got barbecued'?
As long as we're going to talk about misuse of the tools ...
Admin
wasn't sure if it was propel or doctrine or something else, but i knew it was an orm. i think the wtf is to poke a little fun at how using orm's can turn simple queries into long, harder-to-read code... especially when its used in projects that never expect to be ported to other types of databases. but, granted, alex was rather blunt, without pointing out the merits of using an orm (but then i guess it wouldn't be much of a joke then)
Admin
You're half right. Using that interface for a simple query is stupid. Using that interface for a complicated query is ridiculously stupid. It's not bad because it's an abstraction, it's bad because it's a completely worthless abstraction. It doesn't capture anything important about the problem domain. It's exactly as bad as string concatenation except that it's also harder to write!
Compare to something actually useful like DBIx::Class. It's got a similar model where you create an object that represents a query and you successively refine it, but the interface is stated in terms of representing the problem (finding stuff) instead of irrelevant details of an underlying implementation (SQL). The same thing in DBIC would look something like
$schema->resultset('OvertimeAvailability')->search({ -and => [ startDateTime => { '>=', \'NOW()' }, available => 1 ], });and if you want to do successive refinement (very useful with user parameters) you can do it like:
my $results = $schema->resultset('Widgets'); $results = $results->search({ color => 'Blue' }); $results = $results->search({ manf_date => { -between => [ 2006, 2009 ] }); $results = $results->search({ sold => 'N' });which makes $results a ResultSet containing all the blue widgets manufactured between 2006 and 2009 which haven't been sold. See? That's actually accomplishing something.
Admin
There is a lesson to be learned here, if you are open to it: that it is unwise to call others "knuckleheads" based on a reading so superficial that it leaves room for such a fundamental misapprehension. More generally, you might also wish to reflect on how poor manners often cast a harsher light on the party describing than the party described.
Admin
Your "friend" isn't so smart.
"Anyone can make the simple complicated. The secret is making the complicated simple." - Charles Mingus
Admin
TO be fair to him, he was probably a LISP or prolog developer, if you started your career learning LISP, it is difficult to get to know the benefits of procedural programming (as against functional).
Admin
Quite frankly I don't understand what the fuzz is about. The person makes an abstraction of an SQL query, which is a very mature thing to do. The abstraction is clumsy and the implementation leaves much to be desired. But, considering maintainability and security considerations, I prefer code with this SQL generator over inline SQL statements any time.