November 11, 2015 at 12:10 am
Comments posted to this topic are about the item Refactoring SQL Server code
November 11, 2015 at 6:51 am
I hate people refactoring code. They always seem to introduce errors into code that was running OK before they put their mits into it.
November 11, 2015 at 8:23 am
Iwas Bornready (11/11/2015)
I hate people refactoring code. They always seem to introduce errors into code that was running OK before they put their mits into it.
Write unit tests and so if they refactor it and break it they will know.
November 11, 2015 at 3:10 pm
I love refactoring because we can indeed get rid of a huge amount of technical debt, improve performance and scalability of the code, simplify by removed gobs of RBAR, and finally document the code. Code documentation allowed us to drop the time it takes to add a new feature or repair and old one from an average of two days to just two hours and dropped QA rejections from nearly 80% to almost 0.
I will, however, state that no Developer should make the decision as to whether or not to refactor because they're not necessarily aware of the bigger picture in many areas such as how long it will take to sometimes regression test instead of just unit testing and their time isn't actually theirs. Any time spent on refactoring does take away from production development time. Refactoring requires at least one peer review, new testing, etc, etc, etc. Further, the refactoring done might not matter. If you save a second or two on code that's called 40,000 times in 8 hours, that's really cool. If you do the same on something that's only run 10 or 12 times a week, then you've wasted some fairly valuable time.
Refactoring is a good thing but, just like promoting code to production, it does have to be properly controlled.
--Jeff Moden
Change is inevitable... Change for the better is not.
November 11, 2015 at 3:16 pm
Jeff Moden (11/11/2015)
I love refactoring because we can indeed get rid of a huge amount of technical debt, improve performance and scalability of the code, simplify by removed gobs of RBAR, and finally document the code. Code documentation allowed us to drop the time it takes to add a new feature or repair and old one from an average of two days to just two hours and dropped QA rejections from nearly 80% to almost 0.I will, however, state that no Developer should make the decision as to whether or not to refactor because they're not necessarily aware of the bigger picture in many areas such as how long it will take to sometimes regression test instead of just unit testing and their time isn't actually theirs. Any time spent on refactoring does take away from production development time. Refactoring requires at least one peer review, new testing, etc, etc, etc. Further, the refactoring done might not matter. If you save a second or two on code that's called 40,000 times in 8 hours, that's really cool. If you do the same on something that's only run 10 or 12 times a week, then you've wasted some fairly valuable time.
Refactoring is a good thing but, just like promoting code to production, it does have to be properly controlled.
Very true, unless you have a specific task to tidy up some code, you should refactor as you touch things also known as leaving the code a little bit better than before you touched it.
November 11, 2015 at 4:22 pm
Refactoring, at least as used by the defacto canonical book "Refactoring" by Martin Fowler, doesn't mean simply altering code, whether to good effect or not, whether well-written or not.
It's explicitly tied to explicit code patterns, and the application of them to improve existing code. Even then, not all patterns apply in all cases, but context needs to be considered. And the provision of unit tests is a standard practice.
Possibly useful patterns in SQL queries might include reimplementing code that uses cursors, correlated subqueries, or temp tables.
By this definition, Refactoring should be considered a good thing in most cases. But simply ad hoc reimplemention of code as an improvement, whether effective and cost-effective or not, isn't Refactoring.
November 11, 2015 at 4:36 pm
Martin Fowler is pretty clear that refactoring means looking for and cleaning substandard code for example:
http://martinfowler.com/articles/workflowsOfRefactoring/#opp-flow
November 11, 2015 at 7:52 pm
Ed Elliott (11/11/2015)
Martin Fowler is pretty clear that refactoring means looking for and cleaning substandard code for example:http://martinfowler.com/articles/workflowsOfRefactoring/#opp-flow
The thing is, who determines what "substandard" actually is? Based on the post just two above this one, it should NOT be the Developer that makes such a decision. Someone may have heard that correlated subqueries and Temp Tables are a bad thing but nothing could be further from the truth, in many cases. For example, the tried and true method of "Divide'n'Conquer" using a Temp Table to quickly isolate only the rows you need instead of writing a 30 table monster query will frequently be responsible for improving performance quite literally by two or more orders of magnitude. Ironically, the monster query will frequently use dozens and sometimes hundreds more TempDB resources than using a strategically placed Temp Table. If a Developer were to eliminate Temp Table usage by rewriting it all into a single query, killing performance in the process, there'd be a serious calibration session with the Developer in the proverbial woodshed.
The same goes for correlated subqueries. Yes, I agree that having one hanging off of every column in the SELECT list of a query is annoying and should be fixed, but it's not up to the Developer to make that decision because equi-joined correlated subqueries aren't necessarily a problem. Shoot, what do you think the very high performance method of using APPLY is? In many cases, it's not much more than a correlated subquery under the covers.
Even a cursor may be appropriate. Heh... how are you going to refactor it? By using something like sp_MsForEachTable? By using a Recursive CTE? By using a Temp Table and While loop? By using FOR XML PATH to concatenate a bunch of commands? All of THOSE solutions are horribly wrong in that they will fix nothing and only restate the same problem, be even slower, or maybe even break something!
By the same token, a Developer that reimpliments such things as an "aid to understanding" or other supposed "best practice" may be causing an equally large problem.
The bottom line is "IT DEPENDS" and before you go refactoring code on your own, you need to check with the people that are depending on you. If you ARE the TEAM all by your lonesome, then you need to have a serious talk with yourself. 😉
And, remember, I'm both a performance freak and very pro-refactor-for-the-better but you have to use your head.
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 8 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply