Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I need to run to work soon, so this will be brief.

I've only been with the company for a couple weeks. It is a good company, this is a contractor who just happens to be twice my age. I am new to the professional programming field.

This database does exactly what wikipedia says not to do for 1NF. It repeats telephone columns in various tables. That is one strike.

Strike two: Some data is duplicated across tables. Flat out duplicated.

Strike three: Not vital, but he turned all the keys into bigints. All the "FK_" are also nullable. Wtf?

We have not started using this database YET, but there has been a big time crunch to meet the client's needs and timeline since I joined and the current timeline will put it into use, say, tomorrow.

It was okay for me to sit back while he made a mess since I did not have deal with it directly, but it sounds like I'm going to be taking over this section of the code while he's needed to architecture something else.

Any advice would be greatly appreciated. My boss is a great man, but is also very, very busy and more stress is the last thing he needs.

Update:

Sorry this came off as a rant and less of a proper question. I really do appreciate all of the responses and they've helped me start viewing the "problem" more analytically. There's also some benefit to them being generalized in that the same advice will still apply to future "problems."

Thank you.

Second Update:

So I kindly tried to figure out why he did what he did and for the most part he even admitted he didn't have a very good reason. We ended up spending some time and cutting almost all of the duplicated columns. For the rest of the oddities, I can bite my cheek well enough since they're just annoying and won't potentially wreak havoc if every duplicated field isn't updated. All in all a good day.

PS I'm having a hard time choosing a best answer since there are so many good ones.

share|improve this question
19  
My gut says that there is a reason for the database being setup like that. Have you talked to the developer about it? –  Josh K Jun 15 '11 at 14:25
2  
Why the downvotes? Is it so wrong for me to have some emotion in a question asking for advice? –  emragins Jun 15 '11 at 14:48
7  
@emragins - I'm not one of the people who downvoted your question, but I suspect that those who did did so because it reads less like a question and more like a rant. "Do the database design decisions my co-worker made make sense?" makes a better question than "This situation is messed up, how do I deal with this?!". Your question presupposes a problem and asks for vague help when what you really should be asking is if your understanding of the problem is correct. –  Joshua Carmody Jun 15 '11 at 14:51
3  
In the end all that really matters is: can you make the design work as is? If you have problems ask the developer for help. As a new developer the worst thing you can do is prejudge. Once the project is complete you will probably have a better understanding for why some of the decisions were made. –  Chad Jun 15 '11 at 14:58
4  
scream? For this? it can be a LOT worse - just check thedailywtf. –  user1249 Jun 15 '11 at 15:19

9 Answers 9

up vote 13 down vote accepted

"It was okay for me to sit back while he made a mess since I did not have deal with it directly, but it sounds like I'm going to be taking over this section of the code while he's needed to architecture something else."

I hate to say it, but if you had an opportunity to comment way back when, you may have just hit the learning moment where the sad truth is that no area of the code is just one person's problem. In a good team, people SHOULD transition between areas, and that means that the other guy's nightmare can become your nightmare very quickly. The good news is that control gives you the opportunity to make good changes.

I'd suggest the following steps:

  • Talk to the guy that designed it. Ask why he did what he did - what problem was he trying to solve? Did he consider other options? Learn the driving factors to the reason for these decisions.
  • Ponder whether the design is so awful - what is the impact of leaving it alone? While there can be a better way, sometimes the better ways are not so much of a win that they make sense in a short-term sense. And if your group is in a make or break short term crunch, then you may need to trade long term wins like extensibility against short term keep-the-project-going gains.
  • If you can find a better way to solve the problem, and this code has become your problem - then propose the fixes. If you have a decent relationship with the creator, run them by him, but if not - go right to the boss.
  • Since the boss is stressed, skip the saga of woe. Go right to Problem/Why it's Big/Why your Solution is Good. Have time estimates and impacts in mind - if the bad design will cost every developer an extra 4-8 hours per task and the refactoring of the design will take 2 days, it's almost a no brainer. Give the boss the long term perspectives that he doesn't have time to discover for himself.
  • Be ready to make a few sacrifices to have your way. Often in teams I've worked with, the guy who says he's willing to work the weekend to make things right is the guy who gets more play when it comes to decision making. If he is that deeply invested, he's also probably willing to continue to work on his solution if other problems are encountered.
  • Know the other gates to getting what you want - is there both a manager and an architect? Is the original developer such a senior guy that no one has an original thought without running it by him? Know the players and know how they make decisions so you can convince them quickly and efficiently that you need to fix the problem.
share|improve this answer

Of those my biggest concern is the duplicated data. Yes this can be handy to reduce joins and improve performance, however (and it is a critical however), if it is done without an automatic way to maintain the duplicate data, you will have data integrity problems. Depending on the data structure this could be a cascade update and delete or a trigger but it must be in the database (not the application) to be effective. If this was set-up then it is more likely the person knew what he was doing and did it deliberately. If not, you have something to go to your boss with (probably after the intial push to production since it's so close but as soon as possible) to show that the design is flawed and needs to fixed.

If you are going to maintain an imperfect database and wish to move to a better structure over time, the you need to read this book:

http://www.amazon.com/Refactoring-Databases-Evolutionary-paperback-Addison-Wesley/dp/0321774515/ref=sr_1_1?s=books&ie=UTF8&qid=1308151292&sr=1-1

For the future, please bring up concerns with the design as early as possible. Design changes (especially to databases) are much harder to fix the later they are brought up.

share|improve this answer

I agree with the other answerers that there are often good reasons for these design decisions (although they may or may not apply in your case, we can't tell with the info provided), here are some SO questions that pertain to the "strikes" you gave the contractor:

Nullable Foreign Keys:

Denormalization of Data:

share|improve this answer
1  
Refer to a post by name of the poster, not as "above". What happens if you get voted the top answer? –  Jonathan Hobbs Jun 15 '11 at 15:04
    
@Jonathan Hobbs - Good point. I've edited the question. I didn't have a specific answer in mind when I wrote that. –  Joshua Carmody Jun 15 '11 at 15:10
1  
Downvoting to maintain answer order. (Joke) –  Benjol Jun 16 '11 at 6:46
    
@Benjol - <impression voice="Darth Vader">NOOOOOOOOOOOooooooooooooooooooooooooo!</impression> –  Joshua Carmody Jun 16 '11 at 14:32

Without context, it's a bit difficult to answer, but have you considered the reasons behind the duplication and nullable foreign keys before ranting?

I suspect you're correct on the use of bigint fields across the board, but keep in mind at there may be a use for it. If they're holding references to facebook user ids for instance, or twitter post ids.

Nullable foreign key seems perfectly valid to me. Personally, I'm usually suspicious when I see one that is not nullable. In the real world, an order can have a null customer id, or a null billing id. An invoice can be without an order. A payment without an invoice. And so on. Nullables are all over the place, and apps that do not allow for them will typically disallow a whole bunch of situations that shouldn't arise in theory but invariably occur in reality.

Duplicating content in the DB also has its uses. In fact, it's used on this very site. Ever noted how the same user can appear multiple times on a page filled with questions, and have a different reputation in each one? That's because it's stored in a way or another (presumably with the name) to avoid a few queries. Duplication can increase performance.

Other times, duplication is actually necessary. If the name or address of a user changes, or if the name or price of a product changes, the last thing you want is to change prior invoices - which may be needed for tax purposes.

In the end, ask why this/that. You mention another dev twice as old as you are, so I'm assuming you're relatively junior. Ask and check whether the senior guy actually considered a couple of things you didn't even begin to think about. You might be surprised...

share|improve this answer

Changing the key datatypes and enforcing foreign key constraints is pretty easy. A lot of time people don't set those up in the early phase, because they can be a pain in the ass during development.

Phone numbers...Eh. I personally tend to lump phone numbers in their own table, because it allows for an arbitrary number of phone numbers per record, but if you're only allowing one phone per record, there are performance benefits to keeping them with the record.

And sometimes data can be duplicated. Denormalized snowflake-schema tables are the standard for high-volume applications.

In short, make sure you know why before you unload your book learning on the senior dev.

share|improve this answer
1  
+1 for "make sure you know why". Stuff that looks stupid at first glance may not be so stupid after all. –  John Bode Jun 15 '11 at 15:15
1  
@john: My well, "specialty" is maintaining and modernizing legacy code. I have to constantly fight the temptation to say "this is wrong" because sometimes the thing that looks crazy is just an unfortunate compromise dictated by an irrational reality. Of course, sometimes it's just wrong, but you have to understand what they were trying to do, before you can dismiss it. –  Satanicpuppy Jun 15 '11 at 15:21

I highly doubt there is a good reason for the database to be setup like that. Foreign keys being nullable is something that can model a 0 to many relationship, but the fact that EVERY relationship is modeled that way is suspect.

Also, not even in 1NF? Why even use a database you might as well just use flat-files :/

I have seen this kind of incompetence before. No one will be aware of a design problem like this until a couple of years down the road when the data is FUBAR. What I would suggest is that you discretely redesign the database. If you can't get away with it without confronting the senior guy who did this, then discretely talk to him about it. Don't put him on the defensive, make him feel incompetent, and don't approach it in a way that is going to make him feel a personal stake in attempting to defend his FUBAR design, but don't be a pussy about it either. Don't get his boss involved, and don't make him look incompetent. Just sit down with him, redesign it, and give him some pointers. Maybe suggest to him a couple of good resources for learning relational database design fundamentals.

Could be that the guy is an awesome device driver coder or something, but just is completely clueless when it comes to designing a database. Whatever, it doesn't matter. Just get this fixed NOW while you still can.

eta: Good points about this possibly being a reporting database. That is a different scenario. If you have a database that is populated by periodic batch data dumps from another system, and won't be used for atomic inserts, updates, and deletes, then this sort of structure can be more beneficial for reporting purposes.

I get the impression based upon the original post that this is probably not the case. Either way, it is all the more reason to employ my suggestion. Simply go to the dev and discuss this with him. Part of being a valuable member of the team means having a sack and being willing to confront eachother and learn from eachother.

share|improve this answer
4  
I strongly disagree that someone who's only been with the company for only a few weeks should without permission redo work that a more experienced contractor has done. As others have pointed out, there ARE sometimes legitimate reasons for the design decisions you've mentioned. At the very least you need to talk to this person and possibly your supervisor to make sure any changes you want to make are approved and would be as beneficial as you suppose. –  Joshua Carmody Jun 15 '11 at 14:37
1  
-1. As others have pointed out, denormalization is a common technique for optimizing reporting databases. I don't think it's possible to answer the OP's question without knowing what the database will be used for. –  talonx Jun 15 '11 at 14:37
    
After thinking about this and re-reading my suggestion, I agree with you guys. I wouldn't suggest redoing the work without discussing it with him first. –  John Connelly Jun 15 '11 at 14:40
    
If you discuss this with him, either he is going to learn something, or you are...either scenario is a good outcome and it will put you guys on the same page. –  John Connelly Jun 15 '11 at 14:41
    
I wouldn't be suprised if the design problems were not intentional, however. You wouldn't believe the crap that I have seen pass for "database design". –  John Connelly Jun 15 '11 at 14:42

Is it a reporting database? Or maybe a staging schema for a data feed that receives this data in this format and will need to be normalized after it is populate?

Assume he had a reason and find out what it is.

share|improve this answer
2  
"Assume he had a reason and find out what it is." <--- This. Show the guy the respect you want your juniors to show you in 20 years. –  Antony Jun 16 '11 at 5:38
    
Absolutely the correct advice IMO - go with the flow, ask polite questions at the right times, and learn your place and your turf before you go stirring things up. That is, if you want to hold on to this job... –  Vector Jul 7 '11 at 3:37

Sounds like there are a lot of things happening at your component and a lot of demand on employees. Either:

  1. The developer did not have time to make things right and "quick and dirty" solution is getting moved to production (not good, but also common).
  2. Or there is some requirement or reason for the design.

In either case talk to the developer about it. Find out if there are some constraints outside your current vision. If you are going to take over maintenance and there is no problem, then go ahead and make corrections and adjustments as you have time to improve it.

share|improve this answer

whoa there little buddy ... there are valid reasons for all of the gripes you pointed out. denormalization can be perfectly acceptable depending on what your application's data access profile looks like (ie. read-heavy, insert-heavy, reporting, etc.). Sometimes 1nf is too cumbersome

Also, a nullable FK is really nothing out of the ordinary ... again, it all depends on what it's used for.

That being said, this answer is not saying it's right, I have no idea what your app is about. Just saying there's always exceptions to the rule ;-)

share|improve this answer
1  
Okay, thank you. He mentioned something about "reporting" when I brought it up questioningly before. I'll try to find out more about what this it. I really would like to know that there are good reasons it's set up as it is. –  emragins Jun 15 '11 at 14:50
4  
If he mentioned reporting then it makes perfect sense to duplicate certain data because he can avoid JOINs and query a single table or he has to JOIN relatively small amount of tables which still makes it way faster than JOINing every single table there is in order to avoid data duplication. It makes sense what he did, I've seen it MANY times and I'm doing it too. –  Michael J.V. Jun 15 '11 at 15:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.