7 ms·
On the contrary, an approximation of some code I've written: function do_search($query) { if($database->type == "mysql") { return __do_
by Shish2k 13y ago
On the contrary, an approximation of some code I've written:
function do_search($query) {
if($database->type == "mysql") {
return __do_search_stupidly_because_mysql_is_shit($query);
}
else {
return __do_search_sensibly($query);
}
}
- foobarbazqux 13y agoSo a comment could explain why MySQL is shit, what versions, whether it applies to MariaDB, whether there's an open defect, how your algorithm is different, etc. Presumably whoever wrote this code learned some of that metadata, so it's helpful to keep a record of it.
- IanCal 13y agoMost usefully, I'd like to know what the consequences of doing the obvious thing are. Maybe they don't matter any more (high memory usage, cpu use, etc) and we can trim the codebase, or it's still a NEVER CHANGE THIS OR WE LOSE OUR JOBS level issue. I'd much rather see a comment like /* * Inputs of integers near INT_MAX cause significant CPU load * in MYSQL versions 4.2 <= x < 5.8 (latest at time of writing) * Bug tracker http://mysqlbugtracker/issue/4 */ Than lolz_mysql_is_shit(foobar) As a relevant point, we've got a large comment in our codebase linking to issues and discussions of the problem of excel reading csv files containing £ symbols with particular encodings. If you didn't know the issue, having `format_excel_is_dumb(file)` wouldn't help. I am happy classifying comments as "you should have a reason to add it and keep it" rather than "bad code, always remove". Still, I suppose it's hard to generate buzz with an article that says "use your common sense" rather than repeating an old absolutist viewpoint.
- Shish2k 13y agoThose things are commented within the function; but the name of it allows the casual reader to know that madness lies within and they would be happier for not looking at the innards at all :P
- foobarbazqux 13y agoOk, makes sense. By the way, switching on a type code like that is the classic use case for virtual functions. http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism http://sourcemaking.com/refactoring/replace-conditional-with...
- _ZeD_ 13y agohonestly I don't like this approach. In my mind code is for the "how" to do a thing, comments is for the "why". While sometimes some bugs in some library / infrastructure element bites you badly, I still think calling a function "do_workaround_XXX_because_YYY_is_shit" it's not a good practice. And, hey, in comments you can articulate your hate for mysql with more space :D
- ori_b 13y agoWhy is it shit? What do you mean by 'stupidly'? Is this something that you intend to fix later when updating the mysql version, or is it something that will always be stupid? What are the performance impacts? Why not search both databases using the stupid method to avoid two codepaths which might diverge? I prefer to be parsimonious with comments, but strange hacks, workarounds for bugs, and other places where otherwise clean code meets the real world should probably have at least some description of why that particular workaround was chosen, and not some other more obvious option.