2

I work within a team of developers. Everyone is making their changes without carrying too much if the same thing is already implemented in the codebase. This leads to classes constantly growing and to severe duplication.

I want to add line items to class definitions from which a developer could judge what this class has.

  • Would it help?

  • How to do it in Visual Studio?

  • If it wouldn't help, what would be the better alternative to encourage the developers to check if something exists before implementing it?

11
  • 2
    Surely this is not "add-on" territory, just appropriate design, documentation and configuration control?
    – Andrew
    Nov 2, 2012 at 6:22
  • 1
    Thanks for the polite reply... the point of comments is to help to improve the question. As it stands the question is not a good one.
    – Andrew
    Nov 2, 2012 at 6:36
  • @Andrew Thanks for your concern, please guide me towards the appropriate portal where i could get the answer or feel free to edit the question and make it better.
    – MegaMind
    Nov 2, 2012 at 6:40
  • I would not distribute the work in the team such that every body adds code everywhere in the project like that.
    – NoChance
    Nov 2, 2012 at 7:02
  • 4
    Do you mean that people are adding the duplicate functionality to a class? The solution isn't technical but one of education. People should double check and if you institute code reviews you stand a greater chance of spotting duplicate code.
    – ChrisF
    Nov 2, 2012 at 9:14

2 Answers 2

14

There might be several issues which are the cause.

Potential root causes

  • The classes are too generic/too large.

    If there is a class Utils, there are chances that it will grow over time. By laziness, everything which doesn't obviously belong to another class would be put in this one. For example, if somebody writes StringToByteArray method, why bothering to find an appropriate place for it when there is a static Utils class?

    When the class becomes very large, it's nearly impossible to find if something was already implemented, so people would just add their own code to it.

  • The code is undocumented.

    No documentation = difficulty to know what is where = put a new method wherever you want.

  • There are no naming conventions.

    Take an example of a method which converts string to byte array. If you have a naming conventions document specifying that conversions should be named X.ToY, where X is the source type and Y - the destination type, then it's pretty obvious that I should create an extension method public static ToByteArray(this string text). Before creating it, the obvious thing to do is to check (thanks IntelliSense) whether the extension method already exists.

    If there are no naming conventions, you may end up with:

    • this.BytesFromString("Hello")

    • "Hello".ToBytes()

    • "Hello".ConvertToArrayOfBytes()

    • Conversions.FromStringToByteArray("Hello")

    • new TypeConverter<string>("Hello").Convert<byte[]>()

    • and tens of other variants. How would you ever find them?

  • Wrong architecture choices were done originally.

    No need to explain more. If the architecture is a nonsense, it's difficult to follow, which means that one can hardly guess where should be a specific method.

  • Copy-paste is preferred to refactoring.

    When, instead of refactoring, the code is copy-pasted all over the codebase, not only the maintenance is a nightmare (and finding things is a nightmare too), but when you finally refactor the code, you may find yourself moving the slightly different code to the same class, and while two methods are intended to do the same thing, you still keep both methods if they look very different and you don't have time exploring the code inside.

  • Poorly written code with potential bugs is frequent in the codebase.

    Take again the example of a conversion from string to byte array. A developer wants to do the conversion and finds the appropriate method in the codebase: ConvertStringToByteArray. Still, the code inside this method is badly written and it seems that it fails for some strings. Since the product must be delivered in two days, the developer doesn't want to spend time exploring the code (the code inside the method and the code of the callers). Instead, this developer implements his own method, ConvertStringToByteArrayCorrectly.

  • There is a lack of communication between developers or issues with the management.

    Imagine that a developer finds that there is a bug in ConvertStringToByteArray. He could open a ticket or talk directly to the developer who wrote the source code, but the code is written by the lead developer who is an incompetent jerk who hates when somebody notices bugs in his code, and screams every time somebody touches his code without his permission. This lead developer is a brother of the CEO of the company.

    The only way for the developer is to implement his own method.

Conclusion

In all those cases, duplication is unavoidable, and Visual Studio wouldn't help you to solve the problem. The root cause should be solved first, and there are chances that the duplication will disappear.

Examples

Here are two examples which summarize what was said above.

Example 1

You start to work in a company which needs serious changes in the processes, conventions, etc. The deadline is close and everyone is stressed. You are asked to write code where you need to convert a string to an array of bytes before streaming this array to a method. You search the codebase and find four methods which seem to do what you want. They have no documentation whatsoever and are implemented very differently.

You decide to skip the first one: List<byte> ToBytes(string s, int i), because you don't understand what is i, nor why is it creating a list. The second one looks tempting, but when you test it, it "randomly" fails. You try the third one, but it throws Exception every time, this exception being trown at the line 172 of the code, deep inside three embedded loops and four ifs, just before a goto.

Finally, the fourth one looks good, but you should specify the encoding.

From Stack Overflow, you learn that you don't need an encoding for such conversion. Interesting. You decide to refactor the last method, but there are no unit tests whatsoever and the method is called dozens of times through the codebase, so you cannot be sure that it will not break existing functionality.

You decide to ask to the lead developer whether you can refactor the method, but he shouts at you that he's too busy and that you that you shouldn't disturb him with such basic questions: his time is too important for that.

Deadline is close, so you decide to write your own method. You try to understand how things should be named in the codebase, but it seems that there is no logic whatsoever. You end up giving to the new method a name which seems the most appropriate for you.

Example 2

You start to work in a company which care about their employees and the quality of the products. You're asked to write code where you should convert string to byte array.

You're a bit lost, but you know that it's ok to ask questions to your colleagues. You ask one how should you search for existent code through the huge codebase. Your colleague kindly explains that you should read the naming conventions document, and that there is also a wiki explaining the architecture.

From the naming conventions document, it appears that conversions are done through specific factories. Not a wise choice, but the document explains that there are historical reasons for that and that it's too expensive to change the whole codebase now.

You easily discover that the conversion should be done like that:

new Conversions
    .Factory
    .Create<byte[], string>()
    .Convert("Hello")`

You also discover that there is an existent method which does the conversion. Meanwhile, you've read the related Stack Overflow article which says that you don't need to care about encodings, while the existent method deals with them.

You ask your colleagues whether you should modify the existent code. They call the lead developer who explores the code and tells you that you can effectively improve the method.

It's an easy task. The existent code is explicit enough and correctly documented. You quickly modify it and rerun the unit tests to ensure that nothing was broken by the change. Later, QA department confirms that everything is fine.

1
  • 1
    In the "copy-paste" scenario above, there are some static analysis tools that can help identify duplicate/similar code. Take a look at Simian: harukizaemon.com/simian
    – E.Z. Hart
    Nov 2, 2012 at 23:27
2

It sounds like you are already in the situation, so I would suggest:

  1. Use the new feature in Visual Studio 2012 called Code Clone Detection.

  2. Enter all the identical and the top 25% of similar into the backlog.

  3. Make the folks creating the duplicates clean it up.

Do this a few times and people will start to realize they need to be more careful.

If you don't have Ultimate or Premium edition then look at CodeRush or Resharper they have similar features.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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