Thread overview
[Tidbit] making your D code more modular & unittestable
Mar 08, 2017
H. S. Teoh
Mar 08, 2017
Moritz Maxeiner
Mar 08, 2017
XavierAP
Mar 09, 2017
Adam D. Ruppe
Mar 10, 2017
Nick Treleaven
Mar 10, 2017
bpr
Mar 11, 2017
Walter Bright
March 08, 2017
While writing a tool for dissecting various file formats, I found a useful coding pattern that helps your D code be cleaner, more modular, and more easily unittestable.

Initially, I wrote a parser module that directly accessed std.stdio.File to parse file contents.  Pretty standard approach, but it had the disadvantage of needing to do all sorts of shenanigans around rawRead, setting up buffers, managing buffers, etc.. It obscured an otherwise straightforward parser structure and made the code hard to maintain and hard to unittest.

So I came up with an idea to abstract file contents as a random-access range of ubyte with lazy loading, so that I can rewrite file parsing code in the nice, comfortable syntax of array slices without incurring the cost of actually loading the entire file into a ubyte[] buffer.

So I started a new module for wrapping std.stdio.File with a random-access range API. My initial attempt looked something like this:

	module fileslice;
	import std.stdio;
	struct FileSlice {
		File f;
		... // range API
	}

Seems straightforward enough, and helps tidy up the code by localizing buffer management to a single place. But again, the dependency on std.stdio.File made it cumbersome to write unittests. It's better than before, because now I don't have to setup buffers and call rawRead direectly, but I'd still have to create temporary files just to be able to test various parts of the code. It's ugly that unittests would have side-effects on the filesystem.  Plus, I couldn't test error handling directly because I can't insert artificial I/O errors into std.stdio.File (at least, not without truly revolting system-dependent hacks).

My first idea was to change the above code to this:

	module fileslice;
	import std.stdio;
	struct FileSliceImpl(F) {
		F f;	// N.B.: concrete File type abstracted away
		... // range API
	}

	// Preserve compatibility with existing code.
	alias FileSlice = FileSliceImpl!File;

This allows me to substitute std.stdio.File with mock file objects designed to test various aspects of FileSliceImpl, for example:

	unittest {
		struct MockFile {
			ubyte[] data = [1,2,3]; // prebaked data
			void seek(long offset) { ... }
			ubyte[] rawRead(ubyte[] buf) { ... }
			...
		}
		FileSliceImpl!MockFile s;
		// ... do various tests on s
	}

This allowed me to unittest FileSliceImpl without actually writing anything to the filesystem, and prebaking various test data into the code as array literals.

Coming back to the parser module, I now had something like this:

	module parser;
	import fileslice;
	...
	auto parseFile(FileSlice input) {
		... // use nice input[x .. y] syntax, yay!
		return result;
	}

This was nice, but the unittests would need to create FileSlice instances of various sorts, possibly by manually instantiating FileSliceImpl with various mock files to be able to use prebaked ubyte[] data as test cases.  It didn't taste right: that's too much coupling between the parser and fileslice modules. FileSliceImpl really shouldn't be visible outside the fileslice module.

But then inspiration struck: the only thing parseFile() *really* depended on was array-slicing syntax in the input!  The whole issue with lazy loading of data only mattered when the input happens to come from a File. But there's no reason it couldn't come from an array, where there's no need for any buffering or lazy loading! So the dependency on FileSlice is actually redundant. So I rewrote the above code as:

	module parser;
	import std.range.primitives;
	// N.B.: no more import of fileslice needed!!
	...
	auto parseFile(Slice)(Slice input)
		if (isRandomAccessRange!Slice && hasSlicing!Slice &&
		    is(ElementType!Slice : ubyte))
	{
		... // use nice input[x .. y] syntax, yay!
		return result;
	}

Voila! Now my unittests can simply pass ubyte[] data directly into parseFile, and the parser module is now no longer coupled to the fileslice module.  The main program would be what puts the two together. And now parseFile is able to handle data from any source, as long as slicing and random-access are available.  We don't even need to import std.stdio in the parser module!!

And to top that off, now that everything has become templatized, the compiler will do attribute inference for us for free, so we don't even need to deal with ugly attribute soup in the code. Huzzah!

tl;dr: Whenever you have a data structure or a function that depends on a concrete type like File that introduces a dependency between modules, templatize it!  In fact, templatize your code whenever possible -- the more the better.  D templates make your code cleaner, more modular, and more unittestable. And you get attribute inference for free.  Learn to love templates! :-P


T

-- 
I'm still trying to find a pun for "punishment"...
March 08, 2017
On Wednesday, 8 March 2017 at 21:34:19 UTC, H. S. Teoh wrote:
> [...]
>
> tl;dr: Whenever you have a data structure or a function that depends on a concrete type like File that introduces a dependency between modules, templatize it!  In fact, templatize your code whenever possible -- the more the better.  D templates make your code cleaner, more modular, and more unittestable. And you get attribute inference for free.  Learn to love templates! :-P

Interesting read, thank you. Would you consider writing a blog post about this (you know, for community growth enhancement purposes ;p )?
March 08, 2017
On Wednesday, 8 March 2017 at 21:34:19 UTC, H. S. Teoh wrote:
> While writing a tool for dissecting various file formats, I found a useful coding pattern that helps your D code be cleaner, more modular, and more easily unittestable.

https://en.wikipedia.org/wiki/Dependency_inversion_principle

"Abstractions should not depend on details. Details should depend on abstractions."
March 09, 2017
On Wednesday, 8 March 2017 at 21:34:19 UTC, H. S. Teoh wrote:
> So I came up with an idea to abstract file contents as a random-access range of ubyte with lazy loading, so that I can rewrite file parsing code in the nice

Sounds like what the kernel does when you memory-map a file...
March 09, 2017
On 03/08/2017 04:34 PM, H. S. Teoh via Digitalmars-d wrote:
> 	auto parseFile(Slice)(Slice input)
> 		if (isRandomAccessRange!Slice && hasSlicing!Slice &&
> 		    is(ElementType!Slice : ubyte))
> 	{
> 		... // use nice input[x .. y] syntax, yay!
> 		return result;
> 	}
>

Wishlist for D3: Some brilliant form of sugar for declaring a function that takes a range.

March 10, 2017
On Thursday, 9 March 2017 at 20:54:23 UTC, Nick Sabalausky (Abscissa) wrote:
> On 03/08/2017 04:34 PM, H. S. Teoh via Digitalmars-d wrote:
>> 	auto parseFile(Slice)(Slice input)
>> 		if (isRandomAccessRange!Slice && hasSlicing!Slice &&
>> 		    is(ElementType!Slice : ubyte))
>> 	{
>> 		... // use nice input[x .. y] syntax, yay!
>> 		return result;
>> 	}
>>
>
> Wishlist for D3: Some brilliant form of sugar for declaring a function that takes a range.

auto parseFile()(auto input if isRandomAccessRangeOf!ubyte && hasSlicing) {

My spin on an inline parameter constraint idea by Kenji (his doesn't use auto and also has more concept-like sugar):

https://wiki.dlang.org/User:9rnsr/DIP:_Template_Parameter_Constraint

As mentioned in the link, inline constraints can help make more specific error messages when constraints fail.

March 10, 2017
On Friday, 10 March 2017 at 14:58:09 UTC, Nick Treleaven wrote:
> On Thursday, 9 March 2017 at 20:54:23 UTC, Nick Sabalausky
>> Wishlist for D3: Some brilliant form of sugar for declaring a function that takes a range.
>
> auto parseFile()(auto input if isRandomAccessRangeOf!ubyte && hasSlicing) {
>
> My spin on an inline parameter constraint idea by Kenji (his doesn't use auto and also has more concept-like sugar):
>
> https://wiki.dlang.org/User:9rnsr/DIP:_Template_Parameter_Constraint
>
> As mentioned in the link, inline constraints can help make more specific error messages when constraints fail.

That looks like a useful DIP. What has to happen to move it to the DIP repository https://github.com/dlang/DIPs/blob/master/GUIDELINES.md ?




March 10, 2017
On 3/8/2017 1:34 PM, H. S. Teoh via Digitalmars-d wrote:
> [...]

Bingo.

If your algorithmic code includes code to read / write files, that's a big sign things are not properly encapsulated in the code.

Using ranges is a great way to accomplish this, and as you discovered, a bonus is that the range can be replaced with an array for easy unit testing.