How can I explain this is an anti-pattern? [closed]

Question

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 6 years ago.

Improve this question

I recently started at a new job.

The existing system works OK but is poorly designed and hard to maintain, and they are planning to rebuild it in MVC and I fear it will be much worse. (Not because of MVC) I want to discourage the lead dev from using an anti-pattern

Existing issues include

• Hard coded strings in the business logic which are also tied to the interface (e.g. if (x.CustomerType.Contains("Shipping Customer") foobar(); is everywhere
• Some classes map to dbTables directly (which is good). But because the software has to be "easy to add features to ", there is a Sharepoint-esque "DynamicData" table for storing new fields that the customer may require. a la [FieldName] [FieldValue] [FieldType]
• Refusal to use Linq but chains lamda queries for about 2 page widths.
• Exceptions not explicitly thrown or rethrown, but not handled properly (the client can see the call stack because it is just dumped into a popup dialog)
• Refusal to use var but loves the idea of dynamic everything.

What is the best way to go about convincing my lead dev to avoid these horrible methods and not put everything in the generic key-value-pair table in version 2.0? (Considering I am new here and the team lead has been here for many years)

Answer 1

What you are really talking about are not really anti-patterns, but smells. Your question covers multiple completely separate issues and if you google enough, you should find out enough material to argument against your boss. There is also problem of persuading your boss that those are actual problems and need to be addressed. Now, for your problems:

• Having "xxxType" in a class is clear code smell and implies that there should be some kind of object hierarchy. Refactoring is highly encouraged to create good OOP design. If the class can have multiple "types", then Strategy Pattern is good fit.
• This is commonly called Entity-attribute-value model and it has it's merits and problems. It is up to you and your boss to figure out if it fits your application.
• Note that lambda chains are too part of LINQ. And while using LINQ query syntax might create more readable code, some prefer not to use it.
• There is tons of discussion on how exception handling should be done. Google is your friend.
• var and dynamic are two completely different things. And when one uses dynamic, the person should be aware of the downsides it brings. Again, Google should help.

Answer 2

The only way of convincing your team lead to change something is to talk to him. For lots of the problems I guess you don't have to collect "big arguments" why the code should not look like this - most devs know that such code is not clean, but often people just don't care, or go with an attitude of "don't have the time now, will fix this later" (which obviously never happens).

Bad code will only become better when the team has a common understanding of code quality, and the team members are willing to improve this. To get to a common understanding, the team needs to do daily code reviews and discuss the individual issues. And when there are no code reviews now, perhaps the start of the new "V2.0" can be a good occasion to introduce such reviews. Why not suggest this to your team lead?

Answer 3

The biggest smells here are probably MVC and rebuild. It sounds like a stab at achieving buzzword compliance, possibly with the misplaced belief that MVC will sort all the current issues. And on top of that there is of course the whole rewrite from scratch fallacy.

Schema-free has its place, and it sounds like a reasonable choice for your application, but you might want to consider a database that is designed for that use.

As for changing the plans, have you talked to the other developers? A single man's concerns tend not to weigh that hard, especially when that man is the new guy. But if some of the other developers voice similar concerns it becomes a lot harder not to listen. If you can't get any allies there is probably not a lot you can do it.