Timeline for Is it considered an anti pattern to write SQL in the source code?
Current License: CC BY-SA 4.0
43 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Feb 27, 2019 at 20:25 | history | edited | Charles Burns | CC BY-SA 4.0 |
Improve minor examples and formatting.
|
| Apr 10, 2018 at 22:48 | history | edited | Charles Burns | CC BY-SA 3.0 |
Add image from Clean Code by Robert C. Martin
|
| Oct 27, 2017 at 21:26 | history | edited | Charles Burns | CC BY-SA 3.0 |
Minor reformatting
|
| Sep 19, 2017 at 23:11 | comment | added | Andy | Fine, but putting the SQL into a resx is bizarre. A repository with the SQL embedded directly there would be better. But honestly, some sort of ORM is likely fine for the vast majority of cases. | |
| Sep 19, 2017 at 23:05 | comment | added | Charles Burns | @Andy: EF and LINQ are fine for many applications. Both have performance and flexibility limitations so are not suited to all applications. | |
| Sep 19, 2017 at 23:02 | comment | added | Andy | Wow... I don't really see how this is better than just using Linq to SQL/EF. | |
| Sep 19, 2017 at 17:50 | history | edited | Charles Burns | CC BY-SA 3.0 |
Improved formatting
|
| May 17, 2017 at 22:39 | comment | added | Jan Hudec | @SavaB., … and sometimes even non-DAL. There are cases where you are processing the data on the database in a way that is impractical as a stored procedure and end up with algorithm that is part SQL and part something else. And there separating the two won't help anything, because it is one algorithm with one concern. | |
| May 17, 2017 at 22:00 | comment | added | Jan Hudec | @SavaB., but the thing is that the DAL code is part of the same concern as the query, really. | |
| May 17, 2017 at 21:59 | comment | added | Jan Hudec | @SavaB., as an aside, I never said anything about resource files. Putting SQL in resource files has all the disadvantages of both approaches an advantages of neither. The SQL needs to be wrapped in reasonably sized, well named, functions. But these functions may be either stored procedures, or they may be functions in the DAL embedding that SQL. Both approaches are readable, and each has its advantages. But that means embedding SQL is not anti-pattern per se. | |
| May 17, 2017 at 21:59 | comment | added | Sava B. | @JanHudec You sometimes need to see parts of code that have separate concerns, but this doesn't mean that violating the separation of concerns principle is usually a good idea. | |
| May 17, 2017 at 21:57 | comment | added | Jan Hudec | @CharlesBurns, stored procedures are a perfectly legitimate solution to a problem if you have it. Simple applications often don't, though. | |
| May 17, 2017 at 21:55 | comment | added | Jan Hudec | @SavaB., no, you don't always have to see the query and the DAL code at once, but you sometimes do. In the above example, the SQL makes it obvious what is queried from where and how, while “get viewable” does not say all that much. Of course, this means the SQL is embedded in the DAL—so the first example should be pretty much the only content of a function. If that is embedded in 200-line function, it is not readable, but the reason is not embedded SQL, the reason is excessively long function. | |
| May 17, 2017 at 21:51 | comment | added | Charles Burns | I think that stored procedures are a perfectly legitimate solution to the problem. In fact, the stored procedure name is the analog of the descriptive name in a resource file. The point is to avoid having a bunch of SQL strings powdered throughout the application code. I usually use resources because they require no DBA involvement and centralize versioning and maintenance. The best choice depends on one's company culture, core competencies, and project history. | |
| May 17, 2017 at 21:39 | comment | added | Sava B. | @JanHudec You don't always need to see the query and the DAL code at once. The concern of the DAL code is to call a query. It doesn't need to know what the query does, just what it takes and returns. The C# code becomes shorter, more concise, and less poluted. Obviously, one 10k-line file contains the same amount of code as 100 100-line ones, but the monolith file is almost always less maintainable. And sprocs are many time more maintainable that resource files - not only will .sql files likely have auto-formatting, syntax coloring, and autocomplete, their semantics will also be checked. | |
| May 17, 2017 at 21:10 | comment | added | Jan Hudec | @SavaB., no, keystroke is blocker of readability when you need to see the two things at once. The code is neither shorter, nor more concise, when you split it to a stored procedure and function wrapper (remember, comparing the same query with the same binding, written in different styles). As long as you need both parts to understand what it does, splitting it always hurts and never helps. So ultimately it is about where you put the layer division. And simpler applications often don't warrant an abstract API at the database level. | |
| May 17, 2017 at 20:56 | comment | added | Sava B. | @JanHudec 1) A keystroke is a lot of effort, but it's worth it for the sake of readability - it reduces the size of your c# code, removes the need to keep the string literals formatted, improves signal-to-noise ratio.. etc. You can argue that it's a matter of taste, but short, concise code without a bunch of huge string literals is usually more maintainable. 2) The resources as described above are a .NET thing. Your stack of choice probably has something similar. | |
| May 17, 2017 at 19:35 | history | edited | Charles Burns | CC BY-SA 3.0 |
Fixed numbered list formatting
|
| May 17, 2017 at 17:50 | history | edited | Charles Burns | CC BY-SA 3.0 |
Changed name of SQL from "descriptiveName" to "GetViewable"
|
| May 17, 2017 at 17:42 | comment | added | Jan Hudec | @SavaB., 1) that's still a click and a keypress more than in the first case and 2) even then only true with a very specific tooling. In the first case, it is no clicks and no keypresses, in any and every environment. | |
| May 17, 2017 at 14:33 | comment | added | Sava B. | @JanHudec Resources are handled by version control. All you have to do to see the query in the second snippet is click on it, and press f12. | |
| May 17, 2017 at 10:36 | comment | added | Jan Hudec |
The first code is much, much more readable. The later is total pain to read, because you have to go hunting for the SQL to a completely separate system that is not even handled by your version control system, while in the first it is right there where you need it. You made the samples suggestive with the bad naming. Name the variable in the first var getViewableQuery and the stored procedure in the later eyesoreSQL and suddenly you have absolutely no argument why the later should be more readable.
|
|
| May 16, 2017 at 20:01 | history | edited | Charles Burns | CC BY-SA 3.0 |
Edited to address comments to the original answer
|
| May 16, 2017 at 19:48 | history | edited | Charles Burns | CC BY-SA 3.0 |
Edited to address comments to the original answer
|
| May 16, 2017 at 16:03 | comment | added | Sava B. | Css/Js/Hml and templating languages are not the same thing as SQL embedded in c#. SQL in C# is, as far as the compiler is concerned, a meaningless string literal, devoid of any semantics. That, and the fact that this makes the c# file large and difficult to read, is what makes it bad. | |
| May 16, 2017 at 6:58 | comment | added | rleir | "or whichever language we use to development maintainable, quality software" -- who would like to contribute a Java example, 1/ using hibernate, 2/ or a smaller example using a SQL 'mapping' class. See javaworld.com/article/2077706/core-java/… | |
| May 16, 2017 at 2:31 | comment | added | Charles Burns | @jpmc26: I think there is a misunderstanding. I do not recommend use of an ORM (except in applications that need to support many simple queries across more than 3 or 4 DBMSs). I use Dapper, where a 113 line query is just that -- 113 lines of SQL. The resource file was only one suggestion. As a suggestion, I think it does isolate SQL into its own layer. In fact, resource files compile to ordinary C# classes under the hood. Regarding resources being more annoying than strings in code, I respectfully disagree, though I can say the VS resource editor could use some improvement. | |
| May 16, 2017 at 2:15 | comment | added | jpmc26 | Nearly all of your objections are answered simply by isolating the SQL in its own "layer" (a particular namespace containing classes or static methods that contain no more than a simple query execution). ORMs tend to make your concerns here worse, not better; a 113 line query in ORM form is a nightmare. (They don't "avoid the question" at all.) Your "resource file" answer doesn't really help, either; it just moves the problems around. Also, working with embedded resources in a C# project is more annoying than strings in code. | |
| May 15, 2017 at 20:09 | comment | added | Ian Newson | Mixing html, CSS and JavaScript is not an anti pattern. Look at the source for this very page and you'll find quite a bit of JavaScript and some css. Razor is not a language, it's a view engine. That's also a terrible argument anyway, as you can easily counter it by declaring any language a templating language. I hereby c# a templating language! Therefore, use as much SQL in cs files as you like! To be clear, I don't think your sentiment is necessarily wrong but it's too general. The only correct answer for something like this is "it depends". | |
| May 15, 2017 at 19:29 | comment | added | Charles Burns | @RobertHarvey: I am sure our experiences differ, but in my own I have found the abstraction in question to be a win in most cases. I will surely refine my abstraction tradeoffs as I have done up and down for years, and may one day put SQL back in code. YMMV. :) I think microservices can be great, and they too generally separate your SQL details (if SQL is used at all) from core application code. I am less advocating "use my specific method:resources" and more advocating "Generally don't mix oil and water languages." | |
| May 15, 2017 at 19:26 | comment | added | Robert Harvey | This change of philosophy is mirrored in newer development techniques which favor microservices over large, monolithic architectures. Of course, none of this has much to do with sensibly moving all of your strings to a central location, unless that's merely one cog in a very large, impossible to understand wheel. | |
| May 15, 2017 at 19:24 | comment | added | Robert Harvey | Sure. But I've seen this kind of abstraction taken too far, so I'm naturally skeptical. The last overengineered system I had to suffer through had a minimum of six hops (through three different C# solutions and a database) to get from one side to the other. The rewrite I performed made the system much flatter (i.e. less hierarchical, a lesson I learned from Google search), much easier to understand, and much faster to write services in (the time to market for each service was decreased by 80 percent). | |
| May 15, 2017 at 19:20 | comment | added | Charles Burns | @RobertHarvey: Moving the complexity somewhere else is to me the goal. Isn't that why we use function and files and libraries rather than creating one giant text file that represents the whole of program logic? I think it more important in this context than even those examples, because at least with functions and files, all are the same language. With different languages, I feel it is natural to abstract them from each-other and unnatural to concatenate them together. | |
| May 15, 2017 at 19:14 | comment | added | Charles Burns | @RobertHarvey: I think you nailed it. I was a bit too general, among other things. | |
| May 15, 2017 at 19:09 | comment | added | Bryan B | Finally somebody else who actually wants to use Resource files for SQL. | |
| May 15, 2017 at 16:36 | comment | added | Robert Harvey | To be clearer, this isn't an answer to the question "what do I do with my SQL strings," so much as it's an answer to the question "what do I do with any collection of sufficiently complex strings," a question which your answer eloquently addresses. | |
| May 15, 2017 at 16:25 | comment | added | Robert Harvey | This just pushes the complexity somewhere else. Yes, your original code is cleaner, at the expense of adding an indirection that the developer now has to navigate to for maintenance purposes. The real reason you would do this is if each SQL statement was used in multiple places in the code. In that way, it's really no different from any other ordinary refactoring ("Extract method"). | |
| May 15, 2017 at 16:21 | history | edited | Charles Burns | CC BY-SA 3.0 |
Further emphasis of the difference between a templating language and C# (yes, Razor technically ensconces C#, opening necessary but abusable doors)
|
| May 15, 2017 at 16:02 | comment | added | Charles Burns | @jwg: I agree, the post focuses strongly on SQL (vs generically mixing languages). To be fair, the original question is asking specifically about mixing SQL, though many, but not all, of my points apply variously to other mixed languages. | |
| May 15, 2017 at 15:59 | comment | added | Charles Burns | @IanNewson: Razor is a templating language; a language designed to attempt a reasonably elegant mesh of languages, and one of the only templating languages to succeed at the task in my opinion. JavaScript and especially CSS and are expressly designed around HTML, and still their use inside the HTML rather than separate files is considered an anti-pattern in most cases, especially for nontrivial code. | |
| May 15, 2017 at 12:54 | comment | added | Ian Newson | "mixing completely different languages with different syntax in the same file" This is a terrible argument. It would also apply to the Razor view engine, and HTML, CSS and Javascript. Love your graphic novels! | |
| May 15, 2017 at 9:00 | comment | added | jwg | I think this focuses much too much on a bad criticism of having SQL in source code. Having two different languages in the same file is not necessarily terrible. It depends on a lot of things like how they are related and how they are each presented. | |
| May 15, 2017 at 7:01 | history | answered | Charles Burns | CC BY-SA 3.0 |