4 ms·

Without considering the number of submits or file length you will end up with files that are simply longer and edited more getting undue high scores. A single 1

by esrauch 15y ago

Without considering the number of submits or file length you will end up with files that are simply longer and edited more getting undue high scores. A single 100 line file will be scored to be twice as buggy as the exact same code split into two 50 line files. Considering source file lengths roughly follow a Zipfian type distribution, it seems like at least the top 5% length files would get flagged regardless unless the bug distribution is extremely uneven.

po 15y ago
I was thinking the same thing. I was expecting them to analyze the diffs and keep a cache of code blocks that are changing in each bugfix. Keeping track of bugs at the class/function/method level would seem to be more powerful.
- groby_b 15y ago
  Existing research points to "number of bug-fixing commits" as being enough to predict. I would assume that's due to the fact that bugs are not always constrained to a single function, but usually to closely related code. Which often means all that related code is in the same file.
  - jdp23 15y ago
    Indeed. Also, typically code "ownership" is at the file level -- and that's likely to correlate strongly with defects in situations where a peron who hasn't been well trained (or is just a bad programmer) is writing or maintaining the code.
tesseract 15y ago
I've seen a few studies (sorry, no citations handy, if I have time I'll revisit this comment) which concluded that above some fairly low floor, bugginess is strongly positively correlated with file length. Which is to say, always flagging the top 5% longest files as being among the buggiest has a good chance of being the right thing to do.
- esrauch 15y ago
  That could be true, my point was that even if a 1000 line file has a 1% chance of bug-per-line it will be marked "buggy" while a 50 line file with a 10% chance of bug-per-line won't. It makes no sense that if you took that 1000 line file and refactored it into 10 files and those 10 files had the same bugs as the original that the 1000 line file will be flagged as buggy but all 10 of the split files won't.
  - dilap 15y ago
    Not necessarily, because maybe refactoring your code to be clean enough to fit into ten files is also makes it clean enough to remove bugs. And/or hairy code doesn't end up in short files.
    - maaku 15y ago
      Who said anything about refactoring? Chop it up into 10 files verbatim. If that is indeed all it takes to cheat the algorithm, it's not very useful.
      - Lewisham 15y ago
        I'm Chris, author of the post/work. Cheating the algorithm is indeed a possibility and is actually very easy: 1. Just don't file bug tickets. 2. If you do, don't attach the tickets to changes. 3. If you attach the tickets to changes, don't put the "bug" type on the ticket. Doing things to deliberately cheat the algorithm is likely not going to pass code review. One common misconception is that the algorithm is a stick to beat developers with. It really, really isn't, and I go to great lengths to try and make this clear in the internal docs (although the blog post doesn't do this as much). The idea is just to provide another insight into the code, so that devs don't accidentally change something that's a real problem without help. The algorithm is just a tool to help developers understand their code. It's not subjective because different teams use the bug tracker differently, so a score from a file in one project cannot be compared to a file from another project. We mitigate this by only taking off the top 10% of code across the entire codebase, so hopefully we by and large only flag the worst offenders. Part of what I wanted to do was to set up a web app so teams could claim various parts of the code base, and then run the algorithm on a team-by-team basis instead, but I ran out of time on that. I think this is actually one of the nice things about the algorithm, because I don't like the idea of implicitly pitting people or teams against one another. I just like that people can go "Oh hey, this has been a real problem for us, let's tread carefully here."
JesseAldridge 15y ago
You could cut up every file into chunks of 10 or so lines and then run those chunks through the algorithm. Seems like that should yield better results. You ought to be able to pull line numbers from the commits.
nl 15y ago
A single 100 line file will be scored to be twice as buggy as the exact same code split into two 50 line files. I suspect that isn't the case. Subjectively, longer files are harder to understand and in languages like Java (one file = one class, ignore inner classes etc) is likely to represent more complicated and tightly coupled code. If the same file keeps appearing, then split it in half and see what happens...
- cpeterso 15y ago
  The two 50-line files will probably have better internal cohesion, making them easier to test and review than one 100-line file.
Lewisham 15y ago
Hi, I'm Chris, author of the blog post/work. You're on the right lines, and I spent some time thinking about this. There is a couple of things that led me not to concern myself with file length. Before I start, I would note that a file just being big and getting changed isn't a problem: it has to be changed and have a bug ticket with the "bug" classification put against it. 1. As others have stated, the research does strongly point towards LOC as being part of the metric that should be looked at. [1] is a paper that discusses LOC, but it's behind the IEEE pay wall and I wasn't able to find a free version. Sorry. I'm on holiday and a bit pressed for time this morning, but I can root around for more papers if you would like. Pretty much all the models we have for bug prediction will invariably throw LOC into the pot (although as they often do PCA too, one could argue that the LOC going down could be one of the components ;) ). 2. Google, by and large, is an OOP company. The intuitive reason for why LOC leads to defects is that the file is no longer meeting the principle of single responsibility. I would hypothesize that once your file has gone past this, defects are going to start to snowball as your file is on the way to becoming what I called a Gateway file, where large amounts of functionality pass in and out of the file, even if it only does simple stuff. So not only is it going to attract defects because it's long, but it might well attract even more defects because when other functionality is modified, that file has to as well. The added problem is these files resistant to refactoring because of the possibility of breaking so much functionality. A Very Big Philosophical Argument is whether you would put configuration files under this classification, as config files affect all sorts of functionality. Personally, I would, but most of the engineers disagreed with me, so they were left out. 3. In a very unscientific way, I did experiment with all sorts of modifiers to try and even out files that may have been "unfairly" flagged due to some metric like LOC or number of commits or whatever. I eventually abandoned this, not just because I found the results largely unsatisfying, but I realized I really was doing things in an unscientific manner without any real justification for why I was doing it. Without a sound justification for evening it out, which LOC doesn't have, I decided not to do it. Hope this helps a little bit. I very much see your point, and a number of engineers brought this up during user studies, but those big files that were getting flagged really were problems. [1] http://ieeexplore.ieee.org/xpl/freeabs_all.jsp?arnumber=42974 http://ieeexplore.ieee.org/xpl/freeabs_all.jsp?arnumber=4297...

by esrauch 15y ago

po 15y ago

I was thinking the same thing. I was expecting them to analyze the diffs and keep a cache of code blocks that are changing in each bugfix. Keeping track of bugs at the class/function/method level would seem to be more powerful.

groby_b 15y ago
Existing research points to "number of bug-fixing commits" as being enough to predict. I would assume that's due to the fact that bugs are not always constrained to a single function, but usually to closely related code. Which often means all that related code is in the same file.
- jdp23 15y ago
  Indeed. Also, typically code "ownership" is at the file level -- and that's likely to correlate strongly with defects in situations where a peron who hasn't been well trained (or is just a bad programmer) is writing or maintaining the code.

tesseract 15y ago

I've seen a few studies (sorry, no citations handy, if I have time I'll revisit this comment) which concluded that above some fairly low floor, bugginess is strongly positively correlated with file length. Which is to say, always flagging the top 5% longest files as being among the buggiest has a good chance of being the right thing to do.

esrauch 15y ago
That could be true, my point was that even if a 1000 line file has a 1% chance of bug-per-line it will be marked "buggy" while a 50 line file with a 10% chance of bug-per-line won't. It makes no sense that if you took that 1000 line file and refactored it into 10 files and those 10 files had the same bugs as the original that the 1000 line file will be flagged as buggy but all 10 of the split files won't.
- dilap 15y ago
  Not necessarily, because maybe refactoring your code to be clean enough to fit into ten files is also makes it clean enough to remove bugs. And/or hairy code doesn't end up in short files.
  - maaku 15y ago
    Who said anything about refactoring? Chop it up into 10 files verbatim. If that is indeed all it takes to cheat the algorithm, it's not very useful.
    - Lewisham 15y ago
      I'm Chris, author of the post/work. Cheating the algorithm is indeed a possibility and is actually very easy: 1. Just don't file bug tickets. 2. If you do, don't attach the tickets to changes. 3. If you attach the tickets to changes, don't put the "bug" type on the ticket. Doing things to deliberately cheat the algorithm is likely not going to pass code review. One common misconception is that the algorithm is a stick to beat developers with. It really, really isn't, and I go to great lengths to try and make this clear in the internal docs (although the blog post doesn't do this as much). The idea is just to provide another insight into the code, so that devs don't accidentally change something that's a real problem without help. The algorithm is just a tool to help developers understand their code. It's not subjective because different teams use the bug tracker differently, so a score from a file in one project cannot be compared to a file from another project. We mitigate this by only taking off the top 10% of code across the entire codebase, so hopefully we by and large only flag the worst offenders. Part of what I wanted to do was to set up a web app so teams could claim various parts of the code base, and then run the algorithm on a team-by-team basis instead, but I ran out of time on that. I think this is actually one of the nice things about the algorithm, because I don't like the idea of implicitly pitting people or teams against one another. I just like that people can go "Oh hey, this has been a real problem for us, let's tread carefully here."

JesseAldridge 15y ago

You could cut up every file into chunks of 10 or so lines and then run those chunks through the algorithm. Seems like that should yield better results. You ought to be able to pull line numbers from the commits.

nl 15y ago

A single 100 line file will be scored to be twice as buggy as the exact same code split into two 50 line files. I suspect that isn't the case. Subjectively, longer files are harder to understand and in languages like Java (one file = one class, ignore inner classes etc) is likely to represent more complicated and tightly coupled code. If the same file keeps appearing, then split it in half and see what happens...

cpeterso 15y ago
The two 50-line files will probably have better internal cohesion, making them easier to test and review than one 100-line file.

Lewisham 15y ago

Hi, I'm Chris, author of the blog post/work. You're on the right lines, and I spent some time thinking about this. There is a couple of things that led me not to concern myself with file length. Before I start, I would note that a file just being big and getting changed isn't a problem: it has to be changed and have a bug ticket with the "bug" classification put against it. 1. As others have stated, the research does strongly point towards LOC as being part of the metric that should be looked at. [1] is a paper that discusses LOC, but it's behind the IEEE pay wall and I wasn't able to find a free version. Sorry. I'm on holiday and a bit pressed for time this morning, but I can root around for more papers if you would like. Pretty much all the models we have for bug prediction will invariably throw LOC into the pot (although as they often do PCA too, one could argue that the LOC going down could be one of the components ;) ). 2. Google, by and large, is an OOP company. The intuitive reason for why LOC leads to defects is that the file is no longer meeting the principle of single responsibility. I would hypothesize that once your file has gone past this, defects are going to start to snowball as your file is on the way to becoming what I called a Gateway file, where large amounts of functionality pass in and out of the file, even if it only does simple stuff. So not only is it going to attract defects because it's long, but it might well attract even more defects because when other functionality is modified, that file has to as well. The added problem is these files resistant to refactoring because of the possibility of breaking so much functionality. A Very Big Philosophical Argument is whether you would put configuration files under this classification, as config files affect all sorts of functionality. Personally, I would, but most of the engineers disagreed with me, so they were left out. 3. In a very unscientific way, I did experiment with all sorts of modifiers to try and even out files that may have been "unfairly" flagged due to some metric like LOC or number of commits or whatever. I eventually abandoned this, not just because I found the results largely unsatisfying, but I realized I really was doing things in an unscientific manner without any real justification for why I was doing it. Without a sound justification for evening it out, which LOC doesn't have, I decided not to do it. Hope this helps a little bit. I very much see your point, and a number of engineers brought this up during user studies, but those big files that were getting flagged really were problems. [1] http://ieeexplore.ieee.org/xpl/freeabs_all.jsp?arnumber=42974 http://ieeexplore.ieee.org/xpl/freeabs_all.jsp?arnumber=4297...