9

I'm using Visual Studio 2013. I'm reworking some horrible code left by someone else, which uses almost EXCLUSIVELY global variables, and trying to clean that up so I can have each function be properly encapsulated and not cause effects outside of what is passed into (or returned from) it.

Is there any way to easily check the whole project for variables which are defined outside the scope in which they are being used?

I know I can click on a variable, and SHIFT+F12 will find me usages for that individual variable, but I'd like to find all such usages in the whole project, because the problem is REALLY bad... it's not just one or two global variables, I'm talking dozens. Trying to understand the flow of this program and the state of it is enough to make you drink, heavily!

johnnyRose
  • 7,310
  • 17
  • 40
  • 61
eidylon
  • 7,068
  • 20
  • 75
  • 118
  • wouldn't that just be member variables? unless you're also concerned about mutated parameters? – Mark Brackett Jul 02 '15 at 20:34
  • 3
    R# might help you. Not sure if there's a built-in function for it though. @MarkBrackett, the OP is talking about member variables that needn't be member variables, because they're only used inside one method. – bzlm Jul 02 '15 at 20:34
  • @bzlm, almost. They are global variables, that are used to pass state at a global level among dozens of functions. So I'm trying to localize the variable into each function where needed and use parameters instead, so the state of the program and its data is easier to understand, to cleanly encapsulate and do away with all the globals. – eidylon Jul 02 '15 at 20:38
  • 1
    I'll have to look into R#. – eidylon Jul 02 '15 at 20:39
  • Just to clarify, you want to know if there is a quick way to find all the out-of-scope variables, perhaps in a report? A faster way than going to the definition of every variable manually? – ryanyuyu Jul 02 '15 at 20:39
  • Essentially yes... not sure if "out of scope" is the right term, they are technically in scope and accessible, but all the ones that are **defined** out of the scope of where they are being used, yeah. – eidylon Jul 02 '15 at 20:41
  • C# doesn't have "global" variables; they're either static classes or member variables in an instance of a class. Reflection can pull out either/both of those, though I'm not sure what benefit having a list gives you. You still need to Find References and rewrite the calling code....On second thought, I'm going to guess you don't have very many classes to begin with. In that case, using something Reflector will let you trace callers to each of the member variables in a tree. Might be better than going one at a time....maybe? – Mark Brackett Jul 02 '15 at 20:42
  • True... yeah, they are member variables of the class in question, which is the only class in the app. So yeah, they aren't actually global in the proper sense, but since ALL the logic of the program is contained in this one class... they effectively act as such. As for benefits, just trying to get an overview to find how much rewrite exposure remains. – eidylon Jul 02 '15 at 20:44
  • 1
    @MarkBrackett If a class member variable is only used to pass state between two methods in a class, and is sufficiently private, then it can be refactorially rewritten as a method parameter instead. This is what the OP is looking for. No need for reflection; the OP isn't talking about run-time but rather compile-time or design-time. – bzlm Jul 02 '15 at 20:44
  • 1
    Perhaps start a second solution, and copy in single classes then you will get red squigglies everywhere you have out of scope members. – maraaaaaaaa Jul 02 '15 at 20:44
  • Or just copy single classes into another program like Monodevelop or Notepad++ to see which members it doesnt recognize. – maraaaaaaaa Jul 02 '15 at 20:46
  • 1
    You could write a small utility that uses Roslyn to find all instances of this antipattern. Might not be worth it to implement the refactoring in code, but writing something just to find member variables that are referenced in only one method shouldn't be hard. – Asad Saeeduddin Jul 02 '15 at 20:49
  • Could you please paste a minimal example of the kind of thing you're trying to refactor? – Asad Saeeduddin Jul 02 '15 at 21:02
  • 1
    Or comment out the variable you want to check, press Build, the Errors window will tell you where it was used. – CompuChip Jul 02 '15 at 21:44

1 Answers1

1

There should be a tool that already does this in a tree format, but none of the usual suspects seem to have it without the Find Usages dance.

So, here's an attempt using the Reflection APIs by finding all the member variables, then going through the methods and figuring out whether they touch them.

You're basically looking at IL at this point, so unless you want to figure the parsing rules yourself you'll want to use a disassembler like Cecil. I'm using this deprecated single file implementation cause it's easy. Something like a Roslyn analyzer or JustDecompile plugin would be another route, but I have no experience there.

There's bound to be edge cases, and if you're looking for an entire dependency graph the code will be a lot more complex - but for quick and dirty analysis, it should at least get you an overview.

So, basically you use Reflection plus an IL reader to make a map from variable -> methods (you could go the other way too, but that's probably less valuable):

var variables = typeof(SmallBallOfMud).GetFields(BindingFlags.Instance | BindingFlags.NonPublic);
var methods = typeof(SmallBallOfMud).GetMethods(BindingFlags.Instance | BindingFlags.NonPublic);

var d = new Dictionary<string, List<string>>();
foreach (var v in variables) d.Add(v.Name, new List<string>());

// this particular disassembler chokes on externally implemented methods
foreach (var m in methods.Where(m => (m.MethodImplementationFlags | MethodImplAttributes.IL) == 0)) {
    var instructions = MethodBodyReader.GetInstructions(m);

    foreach (var i in instructions) {
        // we'll only check for direct field access here
        var f = i.Operand as FieldInfo; 
        if (f == null) continue;            
        d[f.Name].Add(m.Name);
    }
}

Our result will be like:

state1: Method1 (1), Method2 (1)
state2: Method2 (2)
state3: Method1 (1), Method2 (2)

which is read as "state3" is used by "Method1" once, and "Method2" twice.

Mark Brackett
  • 84,552
  • 17
  • 108
  • 152