Often I end up dealing with code that works but might not be of the highest quality. While quality is subjective I like to use the idea of
"code smell" to convey what I mean, these are a list of indicators that, in total, help to identify code that might benefit from some improvement.
Such smells may include:
- Complex code lacking comments on intended operation
- Code lacking API documentation comments especially for interfaces used outside the local module
- Not following style guide
- Inconsistent style
- Inconsistent indentation
- Poorly structured code
- Overly long functions
- Excessive use of pre-processor
- Many nested loops and control flow clauses
- Excessive numbers of parameters
I am most certainly not alone in using this approach and
Fowler et al have covered this subject in the literature much better than I can here. One point I will raise though is some programmers dismiss code that exhibits these traits as "legacy" and immediately suggest a fresh implementation. There are varying opinions on when a rewrite is the appropriate solution from
never to always but in my experience making the old working code smell nice is almost always less effort and risk than a re-write.
TestsWhen I come across smelly code, and I decide it is worthwhile improving it, I often discover the biggest smell is lack of test coverage. Now do remember this is just one code smell and on its own might not be indicative, my experience is smelly code seldom has effective test coverage while fresh code often does.
Test coverage is generally understood to be the percentage of source code lines and decision paths used when instrumented code is exercised by a set of tests. Like many metrics developer tools produce, "coverage percentage" is often misused by managers as a proxy for code quality. Both
Fowler and
Marick have written about this but sufficient to say that for a developer test coverage is a useful tool but should not be misapplied.
Although refactoring without tests is possible the chances for unintended consequences are proportionally higher. I often approach such a refactor by enumerating all the callers and constructing a description of the used interface beforehand and check that that interface is not broken by the refactor. At which point is is probably worth writing a unit test to automate the checks.
Because of this I have changed my approach to such refactoring to start by ensuring there is at least basic API code coverage. This may not yield the fashionable 85% coverage target but is useful and may be extended later if desired.
It is widely known and equally widely ignored that for maximum effectiveness unit tests must be run frequently and developers take action to rectify failures promptly. A test that is not being run or acted upon is a waste of resources both to implement and maintain which might be better spent elsewhere.
For projects I contribute to frequently I try to ensure that the CI system is running the coverage target, and hence the unit tests, which automatically ensures any test breaking changes will be highlighted promptly. I believe the slight extra overhead of executing the instrumented tests is repaid by having the coverage metrics available to the developers to aid in spotting areas with inadequate tests.
ExampleA short example will help illustrate my point. When a web browser receives an object over HTTP the server can supply a MIME type in a content-type header that helps the browser interpret the resource. However this meta-data is often problematic (sorry that should read "a misleading lie") so the actual content must be examined to get a better answer for the user. This is known as mime sniffing and of course there is a living
specification.
The
source code that provides this API (Linked to it rather than included for brevity) has a few smells:
- Very few comments of any type
- The API are not all well documented in its header
- A lot of global context
- Local static strings which should be in the global string table
- Pre-processor use
- Several long functions
- Exposed API has many parameters
- Exposed API uses complex objects
- The git log shows the code has not been significantly updated since its implementation in 2011 but the spec has.
- No test coverage
While some of these are obvious the non-use of the global string table and the API complexity needed detailed knowledge of the codebase, just to highlight how subjective the sniff test can be. There is also one huge air freshener in all of this which definitely comes from experience and that is the modules author. Their name at the top of this would ordinarily be cause for me to move on, but I needed an example!
First thing to check is the API use
$ git grep -i -e mimesniff_compute_effective_type --or -e mimesniff_init --or -e mimesniff_fini
content/hlcache.c: error = mimesniff_compute_effective_type(handle, NULL, 0,
content/hlcache.c: error = mimesniff_compute_effective_type(handle,
content/hlcache.c: error = mimesniff_compute_effective_type(handle,
content/mimesniff.c:nserror mimesniff_init(void)
content/mimesniff.c:void mimesniff_fini(void)
content/mimesniff.c:nserror mimesniff_compute_effective_type(llcache_handle *handle,
content/mimesniff.h:nserror mimesniff_compute_effective_type(struct llcache_handle *handle,
content/mimesniff.h:nserror mimesniff_init(void);
content/mimesniff.h:void mimesniff_fini(void);
desktop/netsurf.c: ret = mimesniff_init();
desktop/netsurf.c: mimesniff_fini();
This immediately shows me that this API is used in only a very small area, this is often not the case but the general approach still applies.
After a little investigation the usage is effectively that the mimesniff_init API must be called before the mimesniff_compute_effective_type API and the mimesniff_fini releases the initialised resources.
A simple test case was added to cover the API, this exercised the behaviour both when the init was called before the computation and not. Also some simple tests for a limited number of well behaved inputs.
By changing to using the global string table the initialisation and finalisation API can be removed altogether along with a large amount of global context and pre-processor macros. This single change removes a lot of smell from the module and raises test coverage both because the global string table already has good coverage and because there are now many fewer lines and conditionals to check in the mimesniff module.
I stopped the refactor at this point but were this more than an example I probably would have:
- made the compute_effective_type interface simpler with fewer, simpler parameters
- ensured a solid set of test inputs
- examined using a fuzzer to get a better test corpus.
- added documentation comments
- updated the implementation to 2017 specification.
ConclusionThe approach examined here reduce the smell of code in an incremental, testable way to improve the codebase going forward. This is mainly necessary on larger complex codebases where technical debt and bit-rot are real issues that can quickly overwhelm a codebase if not kept in check.
This technique is subjective but helps a programmer to quantify and examine a piece of code in a structured fashion. However it is only a tool and should not be over applied nor used as a metric to proxy for code quality.
Here s my weekly report for week 39 of 2017. In this week I have travelled to
Berlin and caught up on some
podcasts in doing so. I ve also had some trouble with
the RSS feeds on my blog but hopefully this is all fixed
now.
Thanks to Martin Milbret I now have a replacement for my dead
workstation, an HP Z600, and there will be a blog post
about this new set up to come next week. Thanks also to S lvan and a number of
others that made donations towards getting me up and running again. A breakdown
of the donations and expenses can be found at the end of this post.
The title says it all: Firefox is the only piece of software that shows zero sign of improvement in its performance after quadrupling the amount of RAM installed on my 64-bit desktop computer. All other applications show a significant boost in performance. Yet, among all the applications I use, Firefox is the one that most needed this speed boost. To say that this is a major disapointment is an understatement. FFS, Mozilla devs!
For more in-depth explanation on the different ways to encode a routing table
into a trie and a better understanding of radix trees, see
the 
The 
Getting meaningful results is challenging due to the size of the address
space. None of the scenarios have a fallback route and we only measure time for
successful hits
Only 50% of the time is spent in the actual route lookup. The remaining time is
spent evaluating the routing rules (about 30 ns). This ratio is dependent on the
number of routes we inserted (only 1000 in this example). It should be noted the
Here is an approximate breakdown on the time spent:
All kernels are compiled with 
Despite its more complex insertion logic, the IPv4 subsystem is able to insert 2
million routes in less than 10 seconds.
The memory usage is therefore quite predictable and reasonable, as even a small
single-board computer can support several full views (20 MiB for each):
The 
Welcome to gambaru.de. Here is my monthly report that covers what I have been doing for Debian. If you re interested in Java, Games and LTS topics, this might be interesting for you.
Debian Games
Some news about Calibre in Debian: I have been added to the list of maintainers, thanks Martin, and the recent release of 
Calibre 3.5 separates out one module, 
Here is my monthly update covering what I have been doing in the free software world (
In this scenario, instead of doing the traditional transfer from
Netfilter's TPROXY to user space, the kernel directly decapsulates the
HTTP traffic and passes it to BPF rules that can make decisions without
doing expensive context switches or memory copies in the case of simply
wanting to refuse traffic (e.g. issue an HTTP 403 error). This, of
course, requires the 
A new maintenance release 0.3.2.9.1 of 
For people who are visually differently-abled, the above reads
To learn who rules over you, simply find out who you are not allowed to criticize Voltaire wrote this either in late 16th century or early 17th century and those words were as apt in those times, as it is in these turbulent times as well.
Update 05/03 According to @bla these words are attributable to a neo-nazi and apparently a child abuser. While I don t know the context in which it was shared, it describes the environment in which we are perfectly. Please see his comment for a link and better understanding.
The below topic requires a bit of maturity, so if you are easily offended, feel free not to read further.
While this week-end I was supposed to share about the recent Science Day celebrations that we did last week
Would explore it probably next week.
This week the attempt is to share thoughts which had been simmering at the back of my mind for more than 2 weeks or more and whose answers are not clear to me.
My buttons were pressed when Martin f. Kraft 
From the little sharings and interactions I have been part of, I find people take offence at the most innocuous things. For instance, one of the easy routes of not offending anyone is to use self-deprecating humour (or so I thought) either of my race, caste, class or even my issues with weight and each of the above would offend somebody. Charlie Chaplin didn t have those problems. If somebody is from my caste, I m portraying the caste in a certain light, a certain slant. If I m talking about weight issues, then anybody who is like me (fat) feels that the world is laughing at them rather than at me or they will be discriminated against. While I find the last point a bit valid, it leaves with me no tools and no humour. I neither have the observational powers or the skills that 
Martin Pitt won the popular vote.