Detect and fail if any two-way dependencies exist in the project

While this may or may not be perfect, this should improve code quality by preventing new code that is introduced that creates tight coupling. This test is performed when make test is executed.

I avoided retrieving objc_dep in a way that make test would require an internet connection. I also added the executable bit to the objc_dep script, and a source.txt file indicating where the source came from.

Makefile

@@ -21,6 +21,7 @@ build:
 
 test:
 	xcodebuild -scheme Distribution -configuration Debug test
+	./objc_dep/objc_dep.py -t .
 
 uitest:
 	xcodebuild -scheme UITests -configuration Debug test

objc_dep/README.md

@@ -0,0 +1,108 @@
+- 2013-04-17 merged changes from mikkelee who [added](https://github.com/nst/objc_dep/pull/6) to use a single bidirectional arrow for two-way dependencies
+- 2013-02-21 merged changes from jomnius who [added](https://github.com/nst/objc_dep/pull/4) a parameter to exclude directories names by regex
+- 2012-05-11 merged changes from jonreid who [added](https://github.com/nst/objc_dep/pull/3) a parameter to exclude class names by regex
+- 2012-01-11 merged changes from jomnius who [added](https://github.com/nst/objc_dep/pull/1) support for C projects and better handling of categories
+- read [Object graph dependency analysis](http://samuelgoodwin.tumblr.com/post/18345393597/object-graph-dependency-analysis) by Samuel Goodwin
+- see also jominus blog post [Dependency Graph Tool for iOS Projects](http://jomnius.blogspot.com/2012/01/dependency-graph-tool-for-ios-projects.html)
+- yet another use case on vigorouscoding.com [Better get it right the first time](http://www.vigorouscoding.com/2011/12/better-get-it-right-the-first-time/)
+
+# License
+
+    BSD 3-Clause License
+
+    Copyright (c) 2012-2013, Nicolas Seriot All rights reserved.
+
+    Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
+
+    Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
+    Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
+    Neither the name of the Nicolas Seriot nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.
+    THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# Refactoring by graphing class dependencies
+
+### Code design and loose coupling
+
+As developers, we all love clean code, but the fact is that most of the time we're dealing with bad code. It may be recent or legacy code, written by ourselves or by other developers. We can recognize bad code because [code](http://en.wikipedia.org/wiki/Code_smell) [smells](http://www.codinghorror.com/blog/2006/05/code-smells.html). In other words, some heuristics raise questions about code quality. Among thoses we can name dead code, which I already wrote about [here](http://seriot.ch/blog.php?article=20080728) and [here](http://seriot.ch/blog.php?article=20100301), and tight coupling.
+
+Tight coupling describes a system where many components depend on many other components. A tightly coupled code base stinks, the coupling points out that some classes assume too many responsibilities or that a responsability is spread over several classes, rather than having its own class. The opposite, loose coupling, shows a better design which promotes single-responsibility and separation of concerns. Loose coupling makes the code easier to test and maintain.
+
+In Objective-C, reducing coupling generally involves [delegates](http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/ObjectiveC/Articles/ocProtocols.html) and [notifications](http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/Notifications/Introduction/introNotifications.html%23//apple_ref/doc/uid/10000043i).
+
+### Graphing class dependencies
+
+So how do we achieve loose coupling in our own code? Well, at first, we need to get a better idea on the current coupling. Let us define class dependency: _class A depends on B if class A imports class B header_. With such a definition, we can draw a graph of dependencies between classes by considering the Objective-C `#import` directives in each class. We assume here that the files are named according to the classes they contain.
+
+I wrote [objc_dep.py](https://github.com/nst/objc_dep), a Python script which extracts imports from Objective-C source code. The output can then be displayed in [GraphViz](http://www.graphviz.org/) or [OmniGraffle](http://www.omnigroup.com/products/omnigraffle/). You can then see an oriented graph of dependencies between classes. Note that we could also compute metrics on coupling, but it's not the point here.
+
+### Sample usage
+
+How do we get from the dependencies graph to a better design? There's no determinist algorithm and it depends on your project. Let us apply the script on [FSWalker](http://code.google.com/p/fswalker/), a small iPhone file browser I wrote a long time ago.
+
+#### 1. Generate the graph
+
+    $ python objc_dep.py /path/to/FSWalker > fswalker.dot
+
+#### 2. Open it in OmniGraffle
+
+At this point, we see classes as nodes and dependencies as directed edges.
+
+<a href="https://github.com/nst/objc_dep/raw/master/pics/fswalker1.png"><img src="https://github.com/nst/objc_dep/raw/master/pics/fswalker1.png" width="600" /></a>
+
+#### 3. Remove categories
+
+We can safely remove Objective-C [categories](http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/ObjectiveC/Articles/ocCategories.html) from the graph, since referencing categories from many places is not an issue from a design point of view.
+
+<a href="https://github.com/nst/objc_dep/raw/master/pics/fswalker2.png"><img src="https://github.com/nst/objc_dep/raw/master/pics/fswalker2.png" width="600" /></a>
+
+#### 4. Group related classes
+
+Next, we can move the vertices around, try to group classes with a common responsibility into clusters.
+
+<a href="https://github.com/nst/objc_dep/raw/master/pics/fswalker3.png"><img src="https://github.com/nst/objc_dep/raw/master/pics/fswalker3.png" width="600" /></a>
+
+#### 5. Study strange dependencies
+
+The graph now gives a pretty good overview of the overall code structure. The controller objects have been colored in pink, the model objects in yellow and the network part in blue. The graph allows to sport strange dependencies and question the code design. We can see at first glance that FSWalkerAppDelegate has too many dependencies. Specifically we consider:
+
+a) unreferenced classes or clusters
+
+This is probably dead code, which can be removed.
+
+Ok, there are no unreferenced classes here, although you will probably find some in bigger projects. 
+
+b) two-ways references
+
+Maybe one class should not reference another directly, but reference a protocol implemented by the other class instead.
+
+We have two examples of two-ways references here, between HTTPServer and HTTPConnection, and also between RootViewController and FSWalkerAppDelegate. The former is part of [CocoaHTTPServer](http://code.google.com/p/cocoahttpserver/) and is not a design issue in our project. However, the latter is an issue. By looking at the code, we will notice that RootViewController doesn't actually use FSWalkerAppDelegate. The import can thus be safely removed.
+
+c) weird references
+
+Some of the import directives may simply be unnecessary, or reveal design issues.
+
+There is no good reason why FSWalkerAppDelegate would reference FSItem, nor InfoPanelController. Code inspection will reveal that DetailViewController and InfoPanelController should not be referenced by FSWalkerAppDelegate but RootViewController instead. So, here is the final graph. The architecture of FSWalker may still be improved, but you get the idea...
+
+<a href="https://github.com/nst/objc_dep/raw/master/pics/fswalker4.png"><img src="https://github.com/nst/objc_dep/raw/master/pics/fswalker4.png" width="600" /></a>
+
+### Real world usage
+
+Here is the kind of chart you can expect with a 100 classes project.
+
+<a href="https://github.com/nst/objc_dep/raw/master/pics/largerproject.png"><img src="https://github.com/nst/objc_dep/raw/master/pics/largerproject.png" width="600" /></a>
+
+#### Using options
+
+`% python objc_dep.py /path/to/repo -x "(^Internal|secret)" -i subdir1 subdir2 > graph.dot`
+
+Will exclude files with names that begin with "Internal", or contain the word "secret". Additionally all files in folders named subdir1 and subdir2 are ignored.
+
+### Possible improvements
+
+The Cocoa framework enforces the [MVC paradigm](http://developer.apple.com/technologies/mac/cocoa.html), which states (to be short) that model objects and graphical objects should be clearly separated by controller objects. The script could probably be improved by drawing classes which depend on Foundation and classes which depend on AppKit/UIKit with different colors.
+
+### Conclusion
+
+I have found [objc_dep.py](https://github.com/nst/objc_dep) to be definitely useful on small projects as well as bigger ones. It helped me in getting a clear view of code base structure. Spotting strange dependencies allowed me to ask good questions which led to design simplifications. Such a tool could even be integrated into Apple development tools.
+
+Interestingly, the last chart is quite close to the mental representation I have of the code architecture. It reminds me a discussion I had 15 years ago with a friend who was a champion chess player who could play several "blind" chess games simultaneously. He explained then that he saw a blurry chessboard in his head and could move around the pieces as with a small 3D camera. Focusing on a specific piece would then raise awareness of opportunities and risks - dependencies - for this piece. As a side effect, writing this script made me realize that software engineering is pretty similar to chess in this way.

objc_dep/objc_dep.py

@@ -0,0 +1,265 @@
+#!/usr/bin/python
+
+# Nicolas Seriot
+# 2011-01-06 -> 2011-12-16
+# https://github.com/nst/objc_dep/
+
+"""
+Input: path of an Objective-C project
+
+Output: import dependencies Graphviz format, unless --test is passed in which case outputs a successful exit status of 0 if no two-way dependencies are found
+
+Typical usage: $ python objc_dep.py /path/to/project [-x regex] [-i subfolder [subfolder ...]] > graph.dot
+
+The .dot file can be opened with Graphviz or OmniGraffle.
+
+- red arrows: .pch imports
+- blue arrows: two ways imports
+"""
+
+import sys
+import os
+from sets import Set
+import re
+from os.path import basename
+import argparse
+
+local_regex_import = re.compile("^\s*#(?:import|include)\s+\"(?P<filename>\S*)(?P<extension>\.(?:h|hpp|hh))?\"")
+system_regex_import = re.compile("^\s*#(?:import|include)\s+[\"<](?P<filename>\S*)(?P<extension>\.(?:h|hpp|hh))?[\">]")
+objc_extensions = ['.h', '.hh', '.hpp', '.m', '.mm', '.c', '.cc', '.cpp']
+
+def gen_filenames_imported_in_file(path, regex_exclude, system, extensions):
+    for line in open(path):
+        results = re.search(system_regex_import, line) if system else re.search(local_regex_import, line)
+        if results:
+            filename = results.group('filename')
+            extension = results.group('extension') if results.group('extension') else ""
+            if regex_exclude is not None and regex_exclude.search(filename + extension):
+                continue
+            yield (filename + extension) if extension else filename
+
+def dependencies_in_project(path, ext, exclude, ignore, system, extensions):
+    d = {}
+
+    regex_exclude = None
+    if exclude:
+        regex_exclude = re.compile(exclude)
+
+    for root, dirs, files in os.walk(path):
+
+        if ignore:
+            for subfolder in ignore:
+                if subfolder in dirs:
+                    dirs.remove(subfolder)
+
+        objc_files = (f for f in files if f.endswith(ext))
+
+        for f in objc_files:
+
+            filename = f if extensions else os.path.splitext(f)[0]
+            if regex_exclude is not None and regex_exclude.search(filename):
+                continue
+
+            if filename not in d:
+                d[filename] = Set()
+
+            path = os.path.join(root, f)
+
+            for imported_filename in gen_filenames_imported_in_file(path, regex_exclude, system, extensions):
+                if imported_filename != filename and '+' not in imported_filename and '+' not in filename:
+                    imported_filename = imported_filename if extensions else os.path.splitext(imported_filename)[0]
+                    d[filename].add(imported_filename)
+
+    return d
+
+def dependencies_in_project_with_file_extensions(path, exts, exclude, ignore, system, extensions, root_class):
+
+    d = {}
+
+    for ext in exts:
+        d2 = dependencies_in_project(path, ext, exclude, ignore, system, extensions)
+        for (k, v) in d2.iteritems():
+            if not k in d:
+                d[k] = Set()
+            d[k] = d[k].union(v)
+
+    if root_class:
+        def parse_requirements(tree, root, known_deps=[]):
+            next_deps = list(tree[root])
+            new_deps = []
+
+            for dep in next_deps:
+                if dep not in known_deps and dep in tree:
+                    new_deps += parse_requirements(tree, dep, known_deps + next_deps)
+
+            return (new_deps + next_deps)
+
+        requirements = set(parse_requirements(d, root_class))
+
+        return { k: d[k] for k in requirements if k in d}
+
+    return d
+
+def two_ways_dependencies(d):
+
+    two_ways = Set()
+
+    # d is {'a1':[b1, b2], 'a2':[b1, b3, b4], ...}
+
+    for a, l in d.iteritems():
+        for b in l:
+            if b in d and a in d[b]:
+                if (a, b) in two_ways or (b, a) in two_ways:
+                    continue
+                if a != b:
+                    two_ways.add((a, b))
+
+    return two_ways
+
+def untraversed_files(d):
+
+    dead_ends = Set()
+
+    for file_a, file_a_dependencies in d.iteritems():
+        for file_b in file_a_dependencies:
+            if not file_b in dead_ends and not file_b in d:
+                dead_ends.add(file_b)
+
+    return dead_ends
+
+def category_files(d):
+    d2 = {}
+    l = []
+
+    for k, v in d.iteritems():
+        if not v and '+' in k:
+            l.append(k)
+        else:
+            d2[k] = v
+
+    return l, d2
+
+def referenced_classes_from_dict(d):
+    d2 = {}
+
+    for k, deps in d.iteritems():
+        for x in deps:
+            d2.setdefault(x, Set())
+            d2[x].add(k)
+
+    return d2
+
+def print_frequencies_chart(d):
+
+    lengths = map(lambda x:len(x), d.itervalues())
+    if not lengths: return
+    max_length = max(lengths)
+
+    for i in range(0, max_length+1):
+        s = "%2d | %s\n" % (i, '*'*lengths.count(i))
+        sys.stderr.write(s)
+
+    sys.stderr.write("\n")
+
+    l = [Set() for i in range(max_length+1)]
+    for k, v in d.iteritems():
+        l[len(v)].add(k)
+
+    for i in range(0, max_length+1):
+        s = "%2d | %s\n" % (i, ", ".join(sorted(list(l[i]))))
+        sys.stderr.write(s)
+
+def dependencies_in_dot_format(path, exclude, ignore, system, extensions, root_class):
+
+    d = dependencies_in_project_with_file_extensions(path, objc_extensions, exclude, ignore, system, extensions, root_class)
+
+    two_ways_set = two_ways_dependencies(d)
+    untraversed_set = untraversed_files(d)
+
+    category_list, d = category_files(d)
+
+    pch_set = dependencies_in_project(path, '.pch', exclude, ignore, system, extensions)
+
+    #
+
+    sys.stderr.write("# number of imports\n\n")
+    print_frequencies_chart(d)
+
+    sys.stderr.write("\n# times the class is imported\n\n")
+    d2 = referenced_classes_from_dict(d)    
+    print_frequencies_chart(d2)
+
+    #
+
+    l = []
+    l.append("digraph G {")
+    l.append("\tnode [shape=box];")
+
+    for k, deps in d.iteritems():
+        if deps:
+            deps.discard(k)
+
+        if len(deps) == 0:
+            l.append("\t\"%s\" -> {};" % (k))
+
+        for k2 in deps:
+            if not ((k, k2) in two_ways_set or (k2, k) in two_ways_set):
+                l.append("\t\"%s\" -> \"%s\";" % (k, k2))
+
+    l.append("\t")
+    for (k, v) in pch_set.iteritems():
+        l.append("\t\"%s\" [color=red];" % k)
+        for x in v:
+            l.append("\t\"%s\" -> \"%s\" [color=red];" % (k, x))
+
+    l.append("\t")
+    l.append("\tedge [color=blue, dir=both];")
+
+    for (k, k2) in two_ways_set:
+        l.append("\t\"%s\" -> \"%s\";" % (k, k2))
+
+    for k in untraversed_set:
+        l.append("\t\"%s\" [color=gray, style=dashed, fontcolor=gray]" % k)
+
+    if category_list:
+        l.append("\t")
+        l.append("\tedge [color=black];")
+        l.append("\tnode [shape=plaintext];")
+        l.append("\t\"Categories\" [label=\"%s\"];" % "\\n".join(category_list))
+
+    if ignore:
+        l.append("\t")
+        l.append("\tnode [shape=box, color=blue];")
+        l.append("\t\"Ignored\" [label=\"%s\"];" % "\\n".join(ignore))
+
+    l.append("}\n")
+    return '\n'.join(l)
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("-x", "--exclude", nargs='?', default='' ,help="regular expression of substrings to exclude from module names")
+    parser.add_argument("-i", "--ignore", nargs='*', help="list of subfolder names to ignore")
+    parser.add_argument("-s", "--system", action='store_true', default=False, help="include system dependencies")
+    parser.add_argument("-e", "--extensions", action='store_true', default=False, help="print file extensions")
+    parser.add_argument("-r", "--root", default='', help="Class to use as root of dependency graph")
+    parser.add_argument("-t", "--test", action='store_true', default=False, help="Exit with a success status of 0 if no two-way dependencies exist or failure otherwise, instead of outputting a graph")
+    parser.add_argument("project_path", help="path to folder hierarchy containing Objective-C files")
+    args= parser.parse_args()
+
+    if not args.test:
+        print dependencies_in_dot_format(args.project_path, args.exclude, args.ignore, args.system, args.extensions, args.root)
+    else:
+        # Test if two-way dependencies exist. If none do, then exit on success (0), otherwise exit on failure (1)
+        d = dependencies_in_project_with_file_extensions(args.project_path, objc_extensions, args.exclude, args.ignore, args.system, args.extensions, args.root)
+        two_ways_set = two_ways_dependencies(d)
+
+        if len(two_ways_set) == 0:
+            sys.exit(0)
+        else:
+            sys.stderr.write("Error: Found two way dependencies:\n")
+            for (k, k2) in two_ways_set:
+                sys.stderr.write("%s <-> %s\n" % (k, k2))
+            sys.exit(1)
+
+if __name__=='__main__':
+    main()

objc_dep/source.txt

@@ -0,0 +1,3 @@
+Source: https://github.com/nst/objc_dep/
+License: See README.md
+Commit: c73b097d8de0fbca8b9468cf25aaef398d66c68f