Sr. Dev made a database I disagree with. Advice sought

Question

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.

Answer 1

"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.

Answer 2

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 ;-)

Answer 3

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:

• Advantage of nullable Foreign Keys
• Foreign keys and null in MySQL
• Any example of a necessary nullable foreign key?
• Nullable foreign key bad practice

Denormalization of Data:

• Should we denormalize database to improve performance?
• Normalize or denormalize in high traffice websites?

Answer 4

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.

Answer 5

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.

Answer 6

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...

Answer 7

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.

Answer 8

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.

Answer 9

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.