6 ms·
Evolutionary couplings between files reveal poor software design choices
- ColinDabritz 12y agoThis is an amazing approach to making some hard-to-understand aspects of software more visible. This sort of improved information is how software process and tools evolve over time. I look forward to seeing where this project goes.
- barrkel 12y agoA "correctly layered" app with UI view separate from UI logic separate from server-side logic etc. will show up as coupling, if commits are feature oriented. There's certainly a hint as to where to look for bad coupling, but expected "coupling", like tests, need to be discounted.
- masklinn 12y ago> A "correctly layered" app with UI view separate from UI logic separate from server-side logic etc. will show up as coupling, if commits are feature oriented. It would show some coupling commits, but e.g. bug fixes should/would be segregated to the relevant files, not spread across the system.
- nostrademons 12y agoBasically bugfixes would be localized to relevant files, but features would spread across files. The grandparent's making an important point in that you can't design a system such that all possible changes you might want to make are localized to one area of the code. Engineering is about trade-offs: if you rigorously separate view from logic from database, you make it harder to add features that must touch all three. Conversely, if you make each feature its own file and add in hooks to the view/logic/database layer so they call out to plugins, you make it easy to add new features but very difficult to understand what each layer as a whole is doing. The best you can do is choose the ideal architecture for your particular project, in the particular point in time that you're working on it. That's why basically every software system needs to be rewritten as it grows up: the ratio of complete rewrites to new features to bugfixes to maintenance refactorings changes as the system matures and the requirements become more precisely known. It's also why we have a software industry; if there was one ideal way to design a system for all domains and all points in time, someone would go design it and be done with it, and none of us would have jobs.
- wellpast 12y ago> if you rigorously separate view from logic from database, you make it harder to add features that must touch all three I've found the exact opposite of this to be true.
- cema 12y agoI agree! Perhaps the (parent) meant something different, I wonder?
- nostrademons 12y agoFor context, I'm talking about the initial phase of a product's lifecycle, where you are changing the product definition roughly every couple days, the total codebase fits in one person's head, and you spend much more time writing code than reading it. Systems like PHP + "SELECT * FROM database_table" or the MEAN stack, where you use the same data format for both backend storage and UI and intermingle logic with templates, are significantly faster for getting something workable on the screen that users can try out. I've done a complete MVP in 4 days with PHP; a roughly equivalent app in Django (which has some minimal model/view/database separation) took about 2-3 weeks. The fastest I could launch a feature in Google Search that touched both UI and indexing was roughly 6 months; as you'd expect, that has a very rigorous separation of front and back-ends. Now, the PHP solution will quickly become unmaintainable - with the aforementioned 4 day project, I no longer wanted to touch the code after about 3 weeks. But this doesn't matter - it's 10 years later and the software is still serving users, and I long since moved on to bigger and better things. And that's my general point: what's "good" code is context-sensitive. Everybody wants to work on nicely-factored code where you can understand everything, but there is no business to pay you unless you first make something that people want, and oftentimes that takes hundreds of iterations (including many fresh starts where you throw everything away and rebuild from scratch) that are invisible to anyone collecting a paycheck.
- richmarr 12y agoTests should have some coupling, but good tests need to change less frequently than the target code. Bad tests need to change every time. It seems like there's still value to explore there.
- mempko 12y agoI think it is a mistake to think of coupling caused by TDD to be a false positive. What this outlines really is that TDD will force you to edit two files instead of one for many changes. This is a clear indication of how TDD will slow you down.
- pionar 12y agoSo does non-TDD. If you're fixing a bug, your tests should test for conditions that trigger that bug. If you're modifying behavior, you will need to update the tests that break because of that, whether you write tests before or after.
- masklinn 12y ago0. It has nothing to do with TDD 1. It's a tooling artefact, testing systems certainly don't have to mandate split code and tests. Rust's test framework allows tests in the same file as the tested code, the testing guide recommends that unit test live alongside the code they test[0] and the standard library follows this practice[1]. I'm reasonably sure you can also do so in e.g. py.test[2] 2. I'm not convinced editing two files slows you down, most editors and window managers will let you put both files side-by-side and trivially jump between them. Are java developers slowed down by having to jump between files? [0] http://doc.rust-lang.org/guide-testing.html#the-test-module http://doc.rust-lang.org/guide-testing.html#the-test-module [1] https://github.com/rust-lang/rust/blob/4deb27e/src/libcollections/str.rs#L838\ https://github.com/rust-lang/rust/blob/4deb27e/src/libcollec... [2] by marking all python files as "test modules"
- desdiv 12y ago0. It has nothing to do with TDD OP is referring to this line: It also turns out, in both softwares, a majority of the couplings are attributable to Test Driven Design, where a source code is coupled to its test. So these are apparently false-positives I should take care of in the next version of the pipeline.
- jxf 12y ago> This is a clear indication of how TDD will slow you down. Not shown: the part where the critical production bug you introduced was caught by your test suite, thus saving you countless hours of agony, angry customers, and lost revenue.
- __david__ 12y agoThat looks really interesting, and I'd love to run it on my own projects. I'm curious though, why do you think you need to make it a web service? And why tie it to github? I would like to run it on a local git repository and output the results into a local file. That seems like a good small program.
- jlarocco 12y agoI'd also like to run this on our software, but being a web service tied to GitHub kills it for me.
- armish 12y agoright now, it is not really tied to GitHub; but only expects you to give a git repository URL. Coupling it GitHub will allow more information on the system, for example knowledge on commits that resolve issues on the tracker. That is a long-term goal of this project.
- bacongobbler 12y agoIt's not really tied to github. If you look at the source[1] it's just a `git clone` wrapped in a shell script. [1]: https://github.com/armish/evsrc/blob/master/scripts/evSource.sh#L6 https://github.com/armish/evsrc/blob/master/scripts/evSource...
- brysgo1 12y agoI played around with this a bit in June. This is what I managed to come up with in the time I was interested in it: https://bitbucket.org/brysgo/git-coupled https://bitbucket.org/brysgo/git-coupled
- armish 12y agocool! Thanks for sharing the link to your project. I am not really a Ruby expert, so wasn't able to figure out how you calculate the couplings. I will be more than happy to compare the results across these two tools.
- krschultz 12y agoThere was another recent HN post that showed an analysis of IntelliJ's architecture using a source code analyzer [1]. Does anyone have more information on these types of tools? There seems to be a genre of tools that are used to inspect the architecture of a program, and I have no idea where to start learning about them. [1] http://t.co/Ja6uOLRGkQ http://t.co/Ja6uOLRGkQ
- armish 12y agoThanks for the pointer; didn't know about that plug-in. As a long term Intellij Idea user, implementing this as a plug-in to Intellij is one of the things I would like to do in the short term. Would be really great to make the tool let you know about possible coupling as you work on the project. Let me know if you can get your hands on a list of related plug-ins, I am really curios to see them in action.
- RangerScience 12y agoHuh. Could you also use this to flag something for a potential bug, if a historically coupled pair is not coupled in some commit?
- danieltillett 12y agoI actually think this might be the best use case of this proto-tool.
- patmcguire 12y agoAre there undocumented flags that you're running this with? I wind up with a series of graphs too densely packed to make sense of when I run it on angular.
- armish 12y agoThe PDFs that come out of igraph layout are not that pretty. The screenshots I used on the blog post are generated with Cytoscape 3.1.1, which gives better layout and better styling options. For Angular.js, I went with "partialCorrelations_0.3.sif".
- nutmeg 12y agoThe first two times I read this headline I thought it was about fruit fly evolution.
- armish 12y agoI know, right? I was really happy when I learned that this branch of the CS is also referred to as evolutionary, because I was inspired by the evFold approach, which is related to evolution in multiple organisms. This is a bit confusing for people coming from the biological science domain, but also nice that we share some terminology between two fields ;)
- hcarvalhoalves 12y agoVery interesting. Would like to see based on function/class/module instead of file though.
- dkarapetyan 12y agoHold on. I have an interface file "interface.d.ts" and a whole bunch of other files reference it. Whenever I make changes to any files that depend-on/reference that file I of course also make changes to that file. This means that every file in my project is coupled to that file. How is that indicative of good or bad design?
- tracker1 12y agoTight coupling is generally considered bad practice, it leads to more accidental variance and complexity. In general adding either polymorphism, or additional methods to a class are considered safer. I'm not saying in your case it was the wrong choice, or that cleaning up design is bad. Generally if you have to change a whole bunch of related files when you change one, it's an issue with the design.
- emmelaich 12y agoNot sure you can know that without knowing the problem domain. There's a tension there with one-and-one-place-only otherwise known as Don't-Repeat-Yourself.
- adekok 12y agoI'm running it now on a repository with 15 years of history, and ~15K commits. Let's see how it goes. :) One thing I noticed is that the scripts are written to run once, and always do everything. It would be better to have a Makefile and dependencies, so that the you can run it multiple times, and only the changes are updated. I'll see if I can push some fixes to github.
- armish 12y agolet me know if you find points that can be improved; I would be more than glad to pull your changes in to the main repository. Thanks for trying this out.
- grandalf 12y agoI think Rails should rename has_many to couples_many Logical coupling crops up in lots of unexpected places as well: <%= partial :foo %> Partials are functions but with no clear argument signature, so they may be used sloppily with no obvious way of determining what (interface, state expectations) they are coupled to.
- chacham15 12y agoAm I the only one who disagrees with the premise that two files that tend to change a lot together indicate poor software design choices? If you change an API, you will have to change consumers of the API. Does that mean that your code is bad? This exists even in a low level examples: if you change a c++ class, you will need to also change the corresponding header file. Or perhaps, am I misunderstanding the concept?
- lifeisstillgood 12y agoThe author noted there were "legitimate" linkages such as files and test files for those files, header files would be another such case. Eliminating those (which should be moderate to trivial) will leave files that really have no business knowing about each other's internals
- lnanek2 12y agoIdeally, with good encapsulation and individual files keeping to single responsibilities, you would more often get away with just changing implementation and not changing API. Although you could take the size of the change into account to penalize API changes in other files less than implementation changes in both files and ignore header files if you didn't want to clump consumer changes into the same code smell. If the API has to change, you may have exposed too much implementation to the consumers.
- armish 12y agoagreed; those kind of linkages are not necessarily due to bad design but are, on the contrary, intrinsic to the design of the language you code in. Ideally I should consider these kind of things and exclude such pairs (X.cpp <-> X.h) from the final results, but this is still pretty much work in progress. Good catch, though. Thanks.
- briantakita 12y agoThe OP does not really define "bad" software. I'm also not convinced of the concept of "good" vs "bad" software. When reviewing code, we tend to judge how the final output looks based on the reviews' aesthetics. There is almost no emphasis on the process of building the software nor is there much emphasis on how long it takes & how reliable the software is. While aesthetics & clarity are important the notion of "good" or "bad" software depends on the context of the judgement. Is it good/bad for the programmer? Is it good/bad due to the costs of development? Is it good/bad based on it's flexibility toward changing requirements? Is it good/bad based on the flaws in deployed system? Is it good/bad based on the feature velocity? Why is software productivity so difficult to measure? Software is complex & software is created in complex situations. It is tough to get an "apples to apples" comparison when comparing complex contexts. It's like comparing two people. Is one person better than another? Usually it depends on the context...
- tootie 12y agoThat's brilliant. Reminds how did DNA sequencing back in the day. Genetic linkage analysis.
- lnanek2 12y agoThat's pretty cool. There are a lot of tools, both public and private at companies kept as trade secrets to do static analysis of code to pin point potential errors, memory leaks, etc.. This is the first analysis I've seen that actually looks at more than one revision of a project in source control, though. Every other tool basically just analyzes one revision at a time.
- jwmerrill 12y agoThere's some really interesting thinking here, but interpreting this naively would suggest that the perfect software project has only 1 file.
- armish 12y agothat is partially true: single file apps will never have couplings in this manner. One extension to this project would be to find couplings between particular regions of the file and then even single file apps will start falling down. I think it is not true that if you cannot find any coupling using this tool, your software is well-designed; it would just mean this tool is not smart enough to capture those bad designs. Couplings between different regions of the files, however, are relatively harder to find and requires some more thinking in terms of implementation.
- e28eta 12y agoThat's where the Cohesion metric comes into play for OO (and maybe other programming paradigms?). Aka: is each class a single cohesive unit, or do you effectively have two (or more) classes melded together with disjoint data and methods. Something I think is particularly interesting about this approach is that it is language agnostic. It's probably even independent of "programming". It could also be useful for general documents: if I edit wiki page A and always edit B too, should they be the same page instead?
- michaelfeathers 12y agoYou can do some quick and dirty analysis to find classes that change together in the same day. Often you discover that there are some faulty abstractions. http://michaelfeathers.typepad.com/michael_feathers_blog/2011/09/temporal-correlation-of-class-changes.html http://michaelfeathers.typepad.com/michael_feathers_blog/201... I think that this sort of repository analysis is going to be standard practice within the next couple of years.
- pnathan 12y agoThis is similar in aspects to http://google-engtools.blogspot.com/2011/12/bug-prediction-at-google.html http://google-engtools.blogspot.com/2011/12/bug-prediction-a..., which might be a fun set of ideas to link into future iterations.
- a3voices 12y agoThanks, just thinking about coupling made me consider some new design improvements in my code.
- swatow 12y agoperhaps off topic, but evfolds algorithm looks very close to estimating a Markov network. Can anyone comment on how their model differs from a Markov network, and how these differences arise? For reference, given some variables, a Markov network is a parsimonious way to express arbitrary covariance matrices in terms of individual interactions between groups of variables (in this case, pairs of variables). Their approach looks very similar to estimating the Maximum Likelihood or MAP graph.
- ilovecookies 12y agoUse this on the linux source code
- paulsutter 12y ago> I was thinking about writing up a small application paper for this project, but I am really terrible at reading papers from the Computer Science field, let alone writing them. Thank goodness for that. This blog post was so much easier to read then a formal paper. Why are formal papers so tedious to read? Imagine how much time we'd waste as a group if he written this as a paper. Many of us would give up before finding the actual information, and those of us that /did/ identify the actual information would have invested a lot more time than it took to read this excellent blog post.
- evincarofautumn 12y agoI have found that the best papers are hard to read because they’re informationally dense, so you have to slow down to really process every sentence and unpack the author’s thinking in your head—but once you do, you get a lot of knowledge from just a few pages. So it ends up being worthwhile. Average papers are hard to read because they’re trying to emulate the style of the good papers, but without having enough actual content. The length and register of a blog post are definitely a good fit for the average essay. Crappy papers are hard to read because they’re crappy, and recasting them into a different format would reveal that they contain no information at all. :)
- rwallace 12y agoTrue, but it is also true that the best papers would be even better if they were written in less terse and cryptic style so that the same information could be obtained more easily.
- evincarofautumn 12y agoI’m not sure about that. For example, the first time I read them, each of these statements in the declarative specification of Hindley–Milner type inference took me a long time to unpack. x : σ ∈ Γ --------- [Var] Γ ⊢ x : σ Γ ⊢ e₀ : τ → τ′ Γ ⊢ e₁ : τ --------------------------- [App] Γ ⊢ e₀ e₁ : τ′ Γ, x : τ ⊢ e : τ′ ------------------ [Abs] Γ ⊢ λx. e : τ → τ′ Γ ⊢ e₀ : σ Γ, x : σ ⊢ e₁ : τ ----------------------------- [Let] Γ ⊢ let x = e₀ in e₁ : τ Γ ⊢ e : σ′ σ′ ⊑ σ ------------------ [Inst] Γ ⊢ e : σ Γ ⊢ e : σ α ∉ free(Γ) ---------------------- [Gen] Γ ⊢ e : ∀α. σ Each one takes a sentence or two of relatively dense English text to explain: [Var]: If the context indicates that it has a particular type, a variable is inferred to have that type. [App]: If a function is inferred to have some type, and a value is inferred to have the same type as the parameter of that function, then the application of that function to that argument value is inferred to have the type of the result of the function. [Abs]: If the body of a function is inferred to have some type, given an arbitrary type for its parameter, then the type of such a function is a function type from that parameter type to the type inferred for the body. [Let]: If an expression is inferred to have some polymorphic type, and another expression would be inferred to have some monomorphic type given that a particular name were bound to that polymorphic type, then a let-expression binding that name to the former expression within the latter expression would have the same type as inferred for the latter. [Inst]: If an expression has a polymorphic type, and that type is an instance of some more polymorphic type, then the expression can be said to have the more polymorphic type. [Gen]: The type of an expression can be generalised over its free variables into a polymorphic type. But having learned this notation, I can now read and write specifications of type systems with ease, and do so much more quickly and compactly than I could in a more approachable notation. To me, that’s a win. Of course, I’m the sort of person who finds Java programs hard to read because they seem to take so long to say anything. So opinions are going to vary on this!
- wedesoft 12y agoNote to myself: put all source code into one file ;)