Please start any new threads on our new
site at https://forums.sqlteam.com. We've got lots of great SQL Server
experts to answer whatever question you can come up with.
| Author |
Topic |
|
Kristen
Test
22859 Posts |
 Posted - 2013-12-19 : 12:48:52
|
I'm reviewing some code written as part of a 3rd party application.The main part of the application works well, my concern is about the bespoke code written to pull data from our main application database.There is stuff likeWHERE LEFT(MatterNumber, 5) = xxx OK, so the left part of our matter numbers currently has 5 characters, and is followed by a suffix (its something like X9999/ABC). Clearly we will exceed the X9999 range at some point in time.There is another column in the record that contains the X9999 on its own (its the VERY NEXT column in the table in fact - hard to miss!). I have no idea why they didn't use that.Then there isWHERE ...AND MatterType <> 'X' -- Do not include FooBar Cases If I had written that code then the "X" would have been in an external table, probably with other, unrelated, CONFIG data, and I would have used a NOT EXISTS or LEFT JOIN/NULL - then we could add any number of exclusions on MatterType. Gawd knows how many pieces of Bespoke Code this test is duplicated in ...Then there isSELECT ...FROM OurMainApplicationDatabase.dbo.SomeTable my issue here is that if we need to change the name of OurMainApplicationDatabase all the code will need to be changed.Of course, to make that a parameter is going to need dynamic SQL (isn't it??) so maybe this is normal and I should not worry about it?Or is there a way around it so that we can make the database name parametrised?Anyways, my main question, how do you tackle a situation like this?Do I impose my coding standards and quality on the vendor, or live with the code's fragility?(The Vendor is not in our good books, although the application is impressive, it has been installed and running live for months and we don't look any closer to signing it off as "acceptable" as we are lurching from one problem to another; my experience suggests to me that their poor coding style is contributing as they probably put one fire out and, in the process, ignite another half a dozen in other places  |
|
|
tkizer
Almighty SQL Goddess
38200 Posts |
Posted - 2013-12-19 : 13:13:08
|
| I don't think you'll get anywhere if you require a vendor to use your coding standards, unless you are their major customer. We put vendor databases on virtual machines and not on our shared clusters, that way their crappy code cannot impact our internal stuff. Regarding the database name in the code, I despise that due to the code change issue that you mentioned. My most critical application has one of the database names hard-coded in stored procedures. They need many copies of this database everywhere for testing reasons, and I have to use a different SQL instance since we can't change the name. Ideally, I'd use DatabaseName_Perf, DatabaseName_Test, DatabaseName_Dev, etc. But I can't with this application. Sometimes they need multiple Test databases, and I run out of Test SQL instnaces. It's awful. And this is an internally developed application, and I still have no say. I'd have even less say if it were a vendor app though.Tara KizerSQL Server MVP since 2007http://weblogs.sqlteam.com/tarad/ |
 |
|
|
Kristen
Test
22859 Posts |
Posted - 2013-12-19 : 13:32:29
|
Thanks Tara. Good job you don't feel strongly about it, eh?  I'm absolutely with you on this one.The APP has a relatively small, specialist, marketplace so the vendor may appreciate the feedback and benefit from my skills. Who knows whether they buy-in to good-code-quality saving money by being easier to maintain and/or less chance of bugs in the first place ("defensive programming"). Given the varied use of Caps/Lower case, indentation, mix of ISNULL and COALESCE, and so on in the first SProc I looked at I am of the mindset that their code is crappy ... and they do not have the ability to care.Problem is I would like stability for my client, who is paying me a small fortune to try to achieve that for them as for several years NOT having a stable system has been costing them a large fortune.I'll speak to my client and see how they want me to jump. Makes my blood boil though to see such low-grade code thrown-together and sold as a quality solution. |
|
|
|
Kristen
Test
22859 Posts |
Posted - 2013-12-19 : 13:40:46
|
quote:
Originally posted by tkizer
Regarding the database name in the code, I despise that due to the code change issue that you mentioned
Is there a way to program that without using dynamic SQL?VIEWs in the local database that reference the remote database perhaps? That would, at least, allow just the views to be changed for Test/Prod scenarios.Synonyms perhaps? (Never used them, so it might be a stupid suggestion)Perhaps there is a simple parametrised way of doing it in SProcs that I am overlooking? |
|
|
|
tkizer
Almighty SQL Goddess
38200 Posts |
Posted - 2013-12-19 : 13:48:56
|
quote:
Originally posted by Kristen
quote:
Originally posted by tkizer
Regarding the database name in the code, I despise that due to the code change issue that you mentioned
Is there a way to program that without using dynamic SQL?VIEWs in the local database that reference the remote database perhaps? That would, at least, allow just the views to be changed for Test/Prod scenarios.Synonyms perhaps? (Never used them, so it might be a stupid suggestion)Perhaps there is a simple parametrised way of doing it in SProcs that I am overlooking?
Synonyms would work if both databases are on the same instance; synonyms can't be used for an object on a different instance. Other than that though, you'd have to use dynamic SQL if the code is in a stored proc. If the code is in the application instead, then dynamic SQL is not needed. I prefer all data access is through stored procs of course.Tara KizerSQL Server MVP since 2007http://weblogs.sqlteam.com/tarad/ |
|
|
|
Kristen
Test
22859 Posts |
Posted - 2013-12-19 : 19:03:33
|
quote:
Originally posted by tkizer
I prefer all data access is through stored procs of course.
Me too  , and that is indeed what they have built.I'll chat to them about using sp_ExecuteSQL for this job. The actual Sproc is just a single INSERT INTO ... SELECT FROM, it would be trivial to put it in sp_ExecuteSQL with a single parameter for database name.I'm guessing that they haven't thought about it, nor considered the need for a Test/Production option. What I don't know, yet, is how many Sprocs there are that reference our main application database. Hopefully few enough that changing to sp_ExecuteSQL might be realistic. I reckon that all of them are scheduled jobs, so the permissions for SELECT access to the underlying tables should be containable.Thanks, useful advice as always. |
|
|
|
|
|
|
|
|
3rd Party Application Code Quality