5

Due to the way our codebase works, I was advised to not edit certain methods in a shared codebase. Instead, I decided to make a new method that calls the old one and also does the new work our team wanted that method to do. The issue I am foreseeing is not everyone on the team being aware of the new method and still using the old one from the shared codebase.

TLDR:

Shared codebase has SharedCode.Function.

I created OurCode.Function which does some stuff, and then calls SharedCode.Function.

I cannot edit SharedCode.Function or it's containing project.

Is it possible to mark SharedCode.Function as obsolete within our project as a compiler warning somehow or otherwise mark that OurCode.Function essentially replaces it?

EDIT:

Might be pertinent to mention- these are logging functions, so if a dev accidentally calls SharedCode.Function instead of OurCode.Function, the only result is that we capture less data than we desired, it will still compile and run fine. This is a major reason I want to try to make it a compiler warning, so that any dev that doesn't know to use the new function will find out to.

  • This may work https://stackoverflow.com/questions/2490412/adding-obsoleteattribute-to-or-otherwise-greylisting-types-not-under-my-contro – combatc2 Feb 28 '19 at 20:42
  • You may be able to use xml style comments, such that the note will come up in visual studio with intelisense. https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/xml-documentation-comments – dmoore1181 Feb 28 '19 at 20:57
  • Have you looked into writing a [custom analyzer](https://learn.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview?view=vs-2017)? – Kenneth K. Feb 28 '19 at 21:10

2 Answers2

2

The shortest way is by adding the ObsoleteAttribute as an attribute to the method. Make sure to include an appropriate explanation:

[Obsolete("Method1 is deprecated, please use Method2 instead.")]
public void Method1()
{ … }
Joey Phillips
  • 1,543
  • 2
  • 14
  • 22
  • 1
    `I cannot edit SharedCode.Function or it's containing project.` – Kenneth K. Feb 28 '19 at 21:05
  • 1
    I'd do this. I'd create a local method (by local, I mean where you say "OurCode") annotated with this attribute that just calls the shared code method and refactor all of the local code to only call shared code through this new annotated method. – Mr Moose Feb 28 '19 at 22:29
1

I think the adapter pattern could work wonders in this particular scenario. Essentially you end up creating an abstraction between OurCode and SharedCode.

This isolates access to SharedCode functions so that only the adapter can use the shared code's functions. It may end up that some of the functions in the adapter simply provide a pass through, but some of those functions will have extra logic that needs to be applied (such as in the scenario you are asking about), and having the adapter makes it easy for you to enforce that.

All client code is forced to use the adapter since they cannot directly access the shared code.

If you had access to the source code, I would recommend using the Obsolete attribute that the others have pointed out. But since you don't have access to the code base, I think it could be extremely beneficial to have a layer of abstraction between your code and the non-accessible code.

Now obviously I do not have the full scope of your scenario as to whether or not this makes sense to actually implement, so don't drive blindly, but hopefully this gives you some ideas! :)

Reference the gang of four book or see the following resources:

https://martinfowler.com/bliki/RequiredInterface.html

https://www.dofactory.com/net/adapter-design-pattern

Understanding Adapter Pattern

Andrew Meservy
  • 103
  • 2
  • 9
  • 1
    I see where you're coming from, but I'm not sure this will work in my case. I've edited my OP with more info, but the gist is that both SharedCode.Function and OurCode.Function will work properly if used, our version just captures more data to log, so I'm not sure how I would prevent the use of SharedCode since the adapter is not necessary to begin with. – Garrett Cox Feb 28 '19 at 22:27
  • Might I suggest that this be solved as a communication/training with the team? I mean, it seems the most logical route to me since you cannot access the code base in order to flag it as obsolete, and since it sounds like it would be impractical to put an abstraction layer over it to make it truly inaccessible. – Andrew Meservy Mar 01 '19 at 21:57
  • And given that you do code reviews, it wouldn't be too bad to enforce – Andrew Meservy Mar 01 '19 at 22:03
  • Thing is, without access to the code base, or willingness to abstract it away, there aren't many practical options to enforce it. Depending on your code review software, you may be able to set up a rule to catch exactly this scenario and automatically fail it. – Andrew Meservy Mar 01 '19 at 22:04