The title is a bit specific for this blog post, but that's the gist of it: we ended up with a bunch of references to an in-between version of .NET (4.6.1) that was falsely advertising itself as a more optimal candidate for satisfying 4.6.2 dependencies. This is a known issue; there are several links to MS GitHub issues below.
In this blog, I will discuss direct vs. transient dependencies as well as internal vs. runtime dependencies.
If you've run into problems with an application targeted to .NET Framework 4.6.2 that does not compile on certain machines, it's possible that the binding redirects Visual Studio has generated for you use versions of assemblies that aren't installed anywhere but on a machine with Visual Studio installed.
How I solved this issue:
<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
to your project)The product should now run locally and on other machines.
For more details, background and the story of how I ran into and solved this problem, read on.
What do we mean when we say that we "build" an application?
Building is the process of taking a set of inputs and producing an artifact targeted at a certain runtime. Some of these inputs are included directly while others are linked externally.
The machine does exactly what you tell it to, so it's up to you to make sure that your instructions are as precise as possible. However, you also want your application to be flexible so that it can run on as wide an array of environments as possible.
Your source code consists of declarations. We've generally got the direct inputs under control. The code compiles and produces artifacts as expected. It's the external-input declarations where things go awry.
What kind of external inputs does our application have?
How is this stitched together to produce the application that is executed?
The NuGet dependencies are resolved at build time. All resources are pulled and added to the release on the build machine. There are no run-time decisions to make about which versions of which assemblies to use.
Dependencies come in two flavors:
It is with the transient references that we run into issues. The following situations can occur:
An application generally includes an app.config
(desktop applications or services) or web.config
XML file that includes a section where binding redirects are listed. A binding redirect indicates the range of versions that can be mapped (or redirected) to a certain fixed version (which is generally also included as a direct dependency).
A redirect looks like this (a more-complete form is further below):
<bindingRedirect oldVersion="0.0.0.0-4.0.1.0" newVersion="4.0.1.0"/>
When the direct dependency is updated, the binding redirect must be updated as well (generally by updating the maximum version number in the range and the version number of the target of the redirect). NuGet does this for you when you're using package.config
. If you're using Package References, you must update these manually. This situation is currently not so good, as it increases the likelihood that your binding redirects remain too restrictive.
NuGet packages are resolved at build time. These dependencies are delivered as part of the deployment. If they could be resolved on the build machine, then they are unlikely to cause issues on the deployment machine.
Where the trouble comes in is with dependencies that are resolved at execution time rather than build time. The .NET Framework assemblies are resolved in this manner. That is, an application that targets .NET Framework expects certain versions of certain assemblies to be available on the deployment machine.
We mentioned above that the algorithm sometimes chooses the desired version or higher. This is not the case for dependencies that are in the assembly-binding redirects. Adding an explicit redirect locks the version that can be used.
This is generally a good idea as it increases the likelihood that the application will only run in a deployment environment that is extremely close or identical to the development, building or testing environment.
How can we avoid these pesky run-time dependencies? There are several ways that people have come up with, in increasing order of flexibility:
To sum up:
Our application targets .NET Framework (for now). We're looking into .NET Core, but aren't ready to take that step yet.
To sum up the information from above, problems arise when the build machine contains components that are not available on the deployment machine.
How can this happen? Won't the deployment machine just use the best match for the directives included in the build?
Ordinarily, it would. However, if you remember our discussion of assembly-binding redirects above, those are set in stone. What if you included binding redirects that required versions of system dependencies that are only available on your build machine ... or even your developer machine?
We actually discovered an issue in our deployment because the API server was running, but the Authentication server was not. The Authentication server was crashing because it couldn't find the runtime it needed in order to compile its Razor views (it has ASP.Net MVC components). We only discovered this issue on the deployment server because the views were only ever compiled on-the-fly.
To catch these errors earlier in the deployment process, you can enable pre-compiling views in release mode so that the build server will fail to compile instead of a producing a build that will sometimes fail to run.
Add the <MvcBuildViews>true</MvcBuildViews>
to any MVC projects in the PropertyGroup
for the release build, as shown in the example below:
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<DebugType>pdbonly</DebugType>
<Optimize>true</Optimize>
<OutputPath>bin</OutputPath>
<DefineConstants>TRACE</DefineConstants>
<ErrorReport>prompt</ErrorReport>
<WarningLevel>4</WarningLevel>
<LangVersion>6</LangVersion>
<MvcBuildViews>true</MvcBuildViews>
</PropertyGroup>
We mentioned above that NuGet is capable of updating these redirects when the target version changes. An example is shown below. As you can see, they're not very easy to write:
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<runtime>
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
<dependentAssembly>
<assemblyIdentity name="System.Reflection.Extensions" publicKeyToken="B03F5F7F11D50A3A" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-4.0.1.0" newVersion="4.0.1.0"/>
</dependentAssembly>
<!-- Other bindings... -->
</assemblyBinding>
</runtime>
</configuration>
Most bindings are created automatically when MSBuild emits a warning that one would be required in order to avoid potential runtime errors. If you compile with MSBuild in Visual Studio, the warning indicates that you can double-click the warning to automatically generate a binding.
If the warning doesn't indicate this, then it will tell you that you should add the following to your project file:
<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
After that, you can rebuild to show the new warning, double-click it and generate your assembly-binding redirect.
When MSBuild generates a redirect, it uses the highest version of the dependency that it found on the build machine. In most cases, this will be the developer machine. A developer machine tends to have more versions of the runtime targets installed than either the build or the deployment machine.
A Visual Studio installation, in particular, includes myriad runtime targets, including many that you're not using or targeting. These are available to MSBuild but are ordinarily ignored in favor of more appropriate ones.
That is, unless there's a bit of a bug in one or more of the assemblies included with one of the SDKs...as there is with the net461 distribution in Visual Studio 2017.
Even if you are targeting .NET Framework 4.6.2, MSBuild will still sometimes reference assemblies from the 461 distribution because the assemblies are incorrectly marked as having a higher version than those in 4.6.2 and are taken first.
I found the following resources somewhat useful in explaining the problem (though none really offer a solution):
How can you fix the problem if you're affected?
You'll generally have a crash on the deployment server that indicates a certain assembly could not be loaded (e.g. System.Runtime
). If you show the properties for that reference in your web application, do you see the path C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461
somewhere in there? If so, then your build machine is linking in references to this incorrect version. If you let MSBuild generate binding redirects with those referenced paths, they will refer to versions of runtime components that do not generally exist on a deployment machine.
Tips for cleaning up:
C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461
in the output?A sample warning message:
[ResolvePackageFileConflicts] Encountered conflict between 'Platform:System.Collections.dll' and 'CopyLocal:C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\System.Collections.dll'. Choosing 'CopyLocal:C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\System.Collections.dll' because AssemblyVersion '4.0.11.0' is greater than '4.0.10.0'.
As mentioned above, but reiterated here, this what I did to finally stabilize my applications:
<AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
to your project)When you install any update of Visual Studio, it will silently repair these missing files for you. So be aware and check the folder after any installations or upgrades to make sure that the problem doesn't creep up on you again.
Consider the following scenarios:
Under the stresses that come with the combination of these two scenarios, software developers often overlook one critical aspect to a successful, future-proof project: external package-maintenance.
I recently sat down and wrote an email explaining how I go about package-maintenance and thought it would be useful to write up those notes and share them with others.
The tech world moves quickly; new code styles, frameworks and best practices evolve in the blink of an eye. Before you know it, the packages you'd installed the previous year are no longer documented and there aren’t any blogposts describing how to upgrade them to their latest versions. Nightmare.
My general rule of thumb to avoid this ill-fated destiny is to set aside some time each sprint to upgrade packages. The process isn’t really involved, but it can be time-consuming if you upgrade a handful of packages at once and find that one of them breaks your code. You then have to go through each, one by one, downgrade and figure out if it’s the culprit.
My upgrade procedure (in this case using the yarn package manager) is:
yarn outdated
yarn add clean-webpack-plugin@latest
or yarn add clean-webpack-plugin@VERSION_NUMBER
to install a specific versionTom Szpytman is a Software Developer at Encodo and works primarily on the React/Typescript stack
I encountered some curious behavior while writing a service-locator interface (protocol) in Swift. I've reproduced the issue in a stripped-down playground1 and am almost certain I've found a bug in the Swift 3.0.1 compiler included in XCode 8.2.1.
We'll start off with a very basic example, shown below.
The example above shows a very simple function, generic in its single parameter with a required argument label a:
. As expected, the compiler determines the generic type T
to be Int
.
I'm not a big fan of argument labels for such simple functions, so I like to use the _
to free the caller from writing the label, as shown below.
As you can see, the result of calling the function is unchanged.
Let's try calling the function with some other combinations of parameters and see what happens.
If you're coming from another programming language, it might be quite surprising that the Swift compiler happily compiles every single one of these examples. Let's take them one at a time.
int
: This works as expectedodd
: This is the call that I experienced in my original code. At the time, I was utterly mystified how Swift -- a supposedly very strictly typed language -- allowed me to call a function with a single parameter with two parameters. This example's output makes it more obvious what's going on here: Swift interpreted the two parameters as a Tuple. Is that correct, though? Are the parentheses allowed to serve double-duty both as part of the function-call expression and as part of the tuple expression?tuple
: With two sets of parentheses, it's clear that the compiler interprets T
as tuple (Int, Int)
.labels
: The issue with double-duty parentheses isn't limited to anonymous tuples. The compiler treats what looks like two labeled function-call parameters as a tuple with two Ints labeled a:
and b:
.nestedTuple
: The compiler seems to be playing fast and loose with parentheses inside of a function call. The compiler sees the same type for the parameter with one, two and three sets of parentheses.2 I would have expected the type to be ((Int, Int))
instead.complexTuple
: As with tuple
, the compiler interprets the type for this call correctly.The issue with double-duty parentheses seems to be limited to function calls without argument labels. When I changed the function definition to require a label, the compiler choked on all of the calls, as expected. To fix the problem, I added the argument label for each call and you can see the results below.
int
: This works as expectedodd
: With an argument label, instead of inferring the tuple type (Int, Int)
, the compiler correctly binds the label to the first parameter 1
. The second parameter 2
is marked as an error.tuple
: With two sets of parentheses, it's clear that the compiler interprets T
as tuple (Int, Int)
.labels
: This example behaves the same as odd
, with the second parameter b: 2
flagged as an error.nestedTuple
: This example works the same as tuple
, with the compiler ignoring the extra set of parentheses, as it did without an argument label.complexTuple
: As with tuple
, the compiler interprets the type for this call correctly.I claimed above that I was pretty sure that we're looking at a compiler bug here. I took a closer look at the productions for tuples and functions defined in The Swift Programming Language (Swift 3.0.1) manual available from Apple.
First, let's look at tuples:
As expected, a tuple expression is created by surrounding zero or more comma-separated expressions (with optional identifiers) in parentheses. I don't see anything about folding parentheses in the grammar, so it's unclear why (((1)))
produces the same type as (1)
. Using parentheses makes it a bit difficult to see what's going on with the types, so I'm going to translate to C# notation.
()
=> empty tuple3(1)
=> Tuple<int>
((1))
=> Tuple<Tuple<int>>
This seems to be a separate issue from the second, but opposite, problem: instead of ignoring parentheses, the compiler allows one set of parentheses to simultaneously denote the argument clause of a single-arity function call and an argument of type Tuple
encompassing all parameters.
A look at the grammar of a function call shows that the parentheses are required.
Nowhere did I find anything in the grammar that would allow the kind of folding I observed in the compiler, as shown in the examples above. I'm honestly not sure how that would be indicated in grammar notation.
Given how surprising the result is, I can't imagine this is anything but a bug. Even if it can be shown that the Swift compiler is correctly interpreting these cases, it's confusing that the type-inference is different with and without labels.
func test<T>(_ a: T) -> String
{
return String(describing: type(of: T.self))
}
var int = test(1)
var odd = test(1, 2)
var tuple = test((1, 2))
var labels = test(a: 1, b: 2)
var nestedTuple = test((((((1, 2))))))
var complexTuple = test((1, (2, 3)))
The X-Code playground is a very decent REPL for this kind of example. Here's the code I used, if you want to play around on your own.↩
I didn't include the examples, but the type is unchanged with four, five and six sets of parentheses. The compiler treats them as semantically irrelevant, though the Swift grammar doesn't allow for this, as far as I could tell from the BNF in the official manual.↩
This is apparently legal in Swift, but I can't divine its purpose in an actual program↩
Check out two new talks on our web site:
Networking Event: How Encodo builds web applications
At our last networking event, Urs presented our latest tech stack. We've been working productively with this stack for most of this year and feel we've finally stabilized on something we can use for a while. Urs discusses the technologies and libraries (TypeScript, Less, React, MobX) as well as tools (Visual Studio Code, WebStorm).
Since Quino 1.13 came out in December of 2014, we've come a long way. This presentation shows just how far we've come and provides customers with information about the many, many improvements as well as a migration path.
Check out two new talks on our web site:
Microsoft recently published a long blog article Introducing .NET Standard. The author Immo Landwerth appeared on a weekly videocast called The week in .NET to discuss and elaborate. I distilled all of this information into a presentation for Encodo's programmers and published it to our web site, TechTalk: .NET Standard 2.0. I hope it helps!
Also, Sebastian has taken a Tech Talk that he did for a networking event earlier this year, Code Review Best Practices, on the road to Germany, as Die Wahrheit über Code Reviews So klappt's!
Encodo has long been a two-space indent shop. Section 4.1 of the Encodo C# Handbook writes that "[a]n indent is two spaces; it is never a tab.", even though "[t]he official C# standard [...] is four spaces." and that, should you have a problem with that, you should "deal with it."
Although we use our own standards by default, we use a customer's standards if they've defined their own. A large part of our coding is now done with four spaces. Some of us have gotten so accustomed to this that four spaces started looking better than two. That, combined with recent publicity for the topic1, led me to ask the developers at Encodo what they thought.
So, with the rewrite of the Encodo C# Handbook in progress, what will be our recommendation going forward?
Let's summarize the opinions above:
So, we have emphatic arguments against switching to tabs instead of spaces. Although there are good arguments for a 4-space indent, there are also strong arguments for a 2-space indent. There's no real pressure to switch the indent.
Encodo's 2009 recommendation stands: we indent with two spaces. Deal with it.3
# EditorConfig is awesome: http://EditorConfig.org
# top-most EditorConfig file
root = true
# Unix-style newlines with a newline ending every file
[*]
indent_style = space
indent_size = 2
If you're watching Silicon Valley, then you probably already know what prompted this discussion. The most recent episode had Richard of Pied Piper break of a relationship with a girl because she uses spaces instead of tabs.↩
As Richard of Pied Piper recommended, which is just insanity.↩
We use the EditorConfig plugin with all of our IDEs to keep settings for different solutions and products set correctly. The config file for Quino looks like this:↩
We discussed ABD in a recent article ABD: Refactoring and refining an API. To cite from that article,
[...] the most important part of code is to think about how youre writing it and what youre building. You shouldnt write a single line without thinking of the myriad ways in which it must fit into existing code and the established patterns and practices.
With that in mind, I saw another teaching opportunity this week and wrote up my experience designing an improvement to an existing API.
Before we write any code, we should know what we're doing.1
IMetaAspects
) in Quino to add domain-specific metadata (e.g. the IVisibleAspect
controls element visibility)FindOrAddAspect()
. This method does what it advertises: If an aspect with the requested type already exists, it is returned; otherwise, an instance of that type is created, added and returned. The caller gets an instance of the requested type (e.g. IVisibleAspect
).Although we're dealing concretely with aspects in Quino metadata, the pattern and techniques outlined below apply equally well to other, similar domains.
A good example is the IClassCacheAspect
. It exposes five properties, four of which are read-only. You can modify the property (OrderOfMagnitude
) through the interface. This is already not good, as we are forced to work with the implementation type in order to change any property other than OrderOfMagnitude
.
The current way to address this issue would be to make all of the properties settable on the interface. Then we could use the FindOrAddAspect()
method with the IClassCacheAspect
. For example,
var cacheAspect =
Element.Classes.Person.FindOrAddAspect<IClassCacheAspect>(
() => new ClassCacheAspect()
);
cacheAspect.OrderOfMagnitude = 7;
cacheAspect.Capacity = 1000;
For comparison, if the caller were simply creating the aspect instead of getting a possibly-already-existing version, then it would just use an object initializer.
var cacheAspect = Element.Classes.Person.Aspects.Add(
new ClassCacheAspect()
{
OrderOfMagnitude = 7;
Capacity = 1000;
}
}
This works nicely for creating the initial aspect. But it causes an error if an aspect of that type had already been added. Can we design a single method with all the advantages?
A good way to approach a new is to ask: How would we want the method to look if we were calling it?
Element.Classes.Person.SetCacheAspectValues(
a =>
{
a.OrderOfMagnitude = 7;
a.Capacity = 1000;
}
);
If we only want to change a single property, we can use a one-liner:
Element.Classes.Person.SetCacheAspectValues(a => a.Capacity = 1000);
Nice. That's even cleaner and has fewer explicit dependencies than creating the aspect ourselves.
Now that we know what we want the API to look like, let's see if it's possible to provide it. We request an interface from the list of aspects but want to use an implementation to set properties. The caller has to indicate how to create the instance if it doesn't already exist, but what if it does exist? We can't just upcast it because there is no guarantee that the existing aspect is the same implementation.
These are relatively lightweight objects and the requirement above is that the property values on the existing aspect are set on the returned aspect, not that the existing aspect is preserved.
What if we just provided a mechanism for copying properties from an existing aspect onto the new version?
var cacheAspect = new ClassCacheAspect();
var existingCacheAspect =
Element.Classes.Person.Aspects.FirstOfTypeOrDefault<IClassCacheAspect>();
if (existingCacheAspect != null)
{
result.OrderOfMagnitude = existingAspect.OrderOfMagnitude;
result.Capacity = existingAspect.Capacity;
// Set all other properties
}
// Set custom values
cacheAspect.OrderOfMagnitude = 7;
cacheAspect.Capacity = 1000;
This code does exactly what we want and doesn't require any setters on the interface properties. Let's pack this away into the API we defined above. The extension method is:
public static ClassCacheAspect SetCacheAspectValues(
this IMetaClass metaClass,
Action<ClassCacheAspect> setValues)
{
var result = new ClassCacheAspect();
var existingCacheAspect =
metaClass.Aspects.FirstOfTypeOrDefault<IClassCacheAspect>();
if (existingCacheAspect != null)
{
result.OrderOfMagnitude = existingAspect.OrderOfMagnitude;
result.Capacity = existingAspect.Capacity;
// Set all other properties
}
setValues(result);
return result;
}
So that takes care of the boilerplate for the IClassCacheAspect
. It hard-codes the implementation to ClassCacheAspect
, but let's see how big a restriction that is once we've generalized below.
We want to see if we can do anything about generalizing SetCacheAspectValues()
to work for other aspects.
Let's first extract the main body of logic and generalize the aspects.
public static TConcrete SetAspectValues<TService, TConcrete>(
this IMetaClass metaClass,
Action<TConcrete, TService> copyValues,
Action<TConcrete> setValues
)
where TConcrete : new, TService
where TService : IMetaAspect
{
var result = new TConcrete();
var existingAspect = metaClass.Aspects.FirstOfTypeOrDefault<TService>();
if (existingAspect != null)
{
copyValues(result, existingAspect);
}
setValues(result);
return result;
}
This isn't bad, but we've required that the TConcrete
parameter implement a default constructor. Instead, we could require an additional parameter for creating the new aspect.
public static TConcrete SetAspectValues<TService, TConcrete>(
this IMetaClass metaClass,
Func<TConcrete> createAspect,
Action<TConcrete, TService> copyValues,
Action<TConcrete> setValues
)
where TConcrete : TService
where TService : IMetaAspect
{
var result = createAspect();
var existingAspect = metaClass.Aspects.FirstOfTypeOrDefault<TService>();
if (existingAspect != null)
{
copyValues(result, existingAspect);
}
setValues(result);
return result;
}
Wait, wait, wait. We not only don't need to the new
generic constraint, we also don't need the createAspect
lambda parameter, do we? Can't we just pass in the object instead of passing in a lambda to create the object and then calling it immediately?
public static TConcrete SetAspectValues<TService, TConcrete>(
this IMetaClass metaClass,
TConcrete aspect,
Action<TConcrete, TService> copyValues,
Action<TConcrete> setValues
)
where TConcrete : TService
where TService : IMetaAspect
{
var existingAspect = metaClass.Aspects.FirstOfTypeOrDefault<TService>();
if (existingAspect != null)
{
copyValues(aspect, existingAspect);
}
setValues(aspect);
return aspect;
}
That's a bit more logical and intuitive, I think.
We can now redefine our original method in terms of this one:
public static ClassCacheAspect SetAspectValues(
this IMetaClass metaClass,
Action<ClassCacheAspect> setValues)
{
return metaClass.UpdateAspect(
new ClassCacheAspect(),
(aspect, existingAspect) =>
{
result.OrderOfMagnitude = existingAspect.OrderOfMagnitude;
result.Capacity = existingAspect.Capacity;
// Set all other properties
},
setValues
);
}
Can we somehow generalize the copying behavior? We could make a wrapper that expects an interface on the TService
that would allow us to call CopyFrom(existingAspect)
.
public static TConcrete SetAspectValues<TService, TConcrete>(
this IMetaClass metaClass,
TConcrete aspect,
Action<TConcrete> setValues
)
where TConcrete : TService, ICopyTarget
where TService : IMetaAspect
{
return metaClass.UpdateAspect<TService, TConcrete>(
aspect,
(aspect, existingAspect) => aspect.CopyFrom(existingAspect),
setValues
);
}
What does the ICopyTarget
interface look like?
public interface ICopyTarget
{
void CopyFrom(object other);
}
This is going to lead to type-casting code at the start of every implementation to make sure that the other
object is the right type. We can avoid that by using a generic type parameter instead.
public interface ICopyTarget<T>
{
void CopyFrom(T other);
}
That's better. How would we use it? Here's the definition for ClassCacheAspect
:
public class ClassCacheAspect : IClassCacheAspect, ICopyTarget<IClassCacheAspect>
{
public void CopyFrom(IClassCacheAspect otherAspect)
{
OrderOfMagnitude = otherAspect.OrderOfMagnitude;
Capacity = otherAspect.Capacity;
// Set all other properties
}
}
Since the final version of ICopyTarget
has a generic type parameter, we need to adjust the extension method. But that's not a problem because we already have the required generic type parameter in the outer method.
public static TConcrete SetAspectValues<TService, TConcrete>(
this IMetaClass metaClass,
TConcrete aspect,
Action<TConcrete> setValues
)
where TConcrete : TService, ICopyTarget<TService>
where TService : IMetaAspect
{
return metaClass.UpdateAspect(
aspect,
(aspect, existingAspect) => aspect.CopyFrom(existingAspect),
setValues
);
}
Assuming that the implementation of ClassCacheAspect
implements ICopyTarget
as shown above, then we can rewrite the cache-specific extension method to use the new extension method for ICopyTargets
.
public static ClassCacheAspect SetCacheAspectValues(
this IMetaClass metaClass,
Action<ClassCacheAspect> setValues)
{
return metaClass.UpdateAspect<IClassCacheAspect, ClassCacheAspect>(
new ClassCacheAspect(),
setValues
);
}
This is an extension method, so any caller that wants to use its own IClassCacheAspect
could just copy/paste this one line of code and use its own aspect.
This is actually pretty neat and clean:
You would think that would be axiomatic. You'd be surprised.↩
We've been doing more internal training lately and one topic that we've started to tackle is design for architecture/APIs. Even if you're not officially a software architect -- designing and building entire systems from scratch -- every developer designs code, on some level.
[A]lways [B]e [D]esigning
There are broad guidelines about how to format and style code, about how many lines to put in a method, about how many parameters to use, and so on. We strive for Clean Code(tm).
But the most important part of code is to think about how you're writing it and what you're building. You shouldn't write a single line without thinking of the myriad ways in which it must fit into existing code and the established patterns and practices.
We've written about this before, in the two-part series called "Questions to consider when designing APIs" (Part I and Part II). Those two articles comprise a long list of aspects of a design to consider.
First make a good design, then compromise to fit project constraints.
Your project defines the constraints under which you can design. That is, we should still have our designer caps on, but the options available are much more strictly limited.
But, frustrating as that might be, it doesn't mean you should stop thinking. A good designer figures out what would be optimal, then adjusts the solution to fit the constraints. Otherwise, you'll forget what you were compromising from -- and your design skills either erode or never get better.
We've been calling this concept ABD -- Always Be Designing.1 Let's take a closer, concrete look, using a recent issue in the schema migration for Quino. Hopefully, this example illustrates how even the tiniest detail is important.2
We detected the problem when the schema migration generated an invalid SQL statement.
ALTER TABLE "punchclock__timeentry" ALTER COLUMN "personid" SET DEFAULT ;
As you can see, the default value is missing. It seems that there are situations where the code that generates this SQL is unable to correctly determine that a default value could not be calculated.
The code that calculates the default value is below.
result = Builder.GetExpressionPayload(
null,
CommandFormatHints.DefaultValue,
new ExpressionContext(prop),
prop.DefaultValueGenerator
);
To translate, there is a Builder
that produces a payload. We're using that builder to get the payload (SQL, in this case) that corresponds to the DefaultValueGenerator
expression for a given property, prop
.
This method is an extension method of the IDataCommandBuilder
, reproduced below in full, with additional line-breaks for formatting:
public static string GetExpressionPayload<TCommand>(
this IDataCommandBuilder<TCommand> builder,
[CanBeNull] TCommand command,
CommandFormatHints hints,
IExpressionContext context,
params IExpression[] expressions)
{
if (builder == null) { throw new ArgumentNullException("builder"); }
if (context == null) { throw new ArgumentNullException("context"); }
if (expressions == null) { throw new ArgumentNullException("expressions"); }
return builder.GetExpressionPayload(
command,
hints,
context,
expressions.Select(
e => new ExecutableQueryItem<IExecutableExpression>(new ExecutableExpression(e))
)
);
}
This method does no more than to package each item in the expressions
parameter in an ExecutableQueryItem
and call the interface method.
The problem isn't immediately obvious. It stems from the fact that each ExecutableQueryItem
can be marked as Handled
. The extension method ignores this feature, and always returns a result. The caller is unaware that the result may correspond to an only partially handled expression.
Our first instinct is, naturally, to try to figure out how we can fix the problem.3 In the code above, we could keep a reference to the executable items and then check if any of them were unhandled, like so:
var executableItems = expressions.Select(
e => new ExecutableQueryItem<IExecutableExpression>(new ExecutableExpression(e))
);
var result = builder.GetExpressionPayload(command, hints, context, executableItems);
if (executableItems.Unhandled().Any())
{
// Now what?
}
return result;
}
We can detect if at least one of the input expressions could not be mapped to SQL. But we don't know what to do with that information.
null
? What can we return to indicate that the input expressions could not be mapped? Here we have the same problem as with throwing an exception: all callers assume that the result can be mapped.So there's no quick fix. We have to change an API. We have to design.
As with most bugs, the challenge lies not in knowing how to fix the bug, but in how to fix the underlying design problem that led to the bug. The problem is actually not in the extension method, but in the method signature of the interface method.
Instead of a single result, there are actually two results for this method call:
Instead of a Get
method, this is a classic TryGet
method.
If this code is already in production, then you have to figure out how to introduce the bug fix without breaking existing code. If you already have consumers of your API, you can't just change the signature and cause a compile error when they upgrade. You have to decorate the existing method with [Obsolete]
and make a new interface method.
So we don't change the existing method and instead add the method TryGetExpressionPayload()
to IDataCommandBuilder
.
Now, let's figure out what the parameters are going to be.
The method called by the extension method above has a slightly different signature.5
string GetExpressionPayload(
[CanBeNull] TCommand command,
CommandFormatHints hints,
[NotNull] IExpressionContext context,
[NotNull] IEnumerable<ExecutableQueryItem<IExecutableExpression>> expressions
);
That last parameter is a bit of a bear. What does it even mean? The signature of the extension method deals with simple IExpression
objects -- I know what those are. But what are ExecutableQueryItems
and IExecutableExpressions
?
As an author and maintainer of the data driver, I know that these objects are part of the internal representation of a query as it is processed. But as a caller of this method, I'm almost never going to have a list of these objects, am I?
Let's find out.
Me: Hey, ReSharper, how many callers of that method are there in the entire Quino source? ReSharper: Just one, Dave.6
So, we defined an API with a signature that's so hairy no-one calls it except through an extension method that makes the signature more palatable. And it introduces a bug. Lovely.
We've now figured out that our new method should accept a sequence of IExpression
objects instead of ExecutableQueryItem
objects.
How's the signature looking so far?
bool TryGetExpressionPayload(
[CanBeNull] TCommand command,
CommandFormatHints hints,
[NotNull] IExpressionContext context,
[NotNull] IEnumerable<IExpression> expressions,
out string payload
);
Not quite. There are two things that are still wrong with this signature, both important.
One problem is that the rest of the IDataCommandBuilder<TCommand>
deals with a generic payload type and this method only works for builders where the target representation is a string. The Mongo driver, for example, uses MongoStorePayload
and MongoRetrievePayload
objects instead of strings and throws a NotSupportedException
for this API.
That's not very elegant, but the Mongo driver was forced into that corner by the signature. Can we do better? The API would currently require Mongo to always return false
because our Mongo driver doesn't know how to map anything to a string. But it could map to one of the aforementioned object representations.
If we change the out
parameter type from a string
to an object
, then any driver, regardless of payload representation, has at least the possibility of implementing this API correctly.
Another problem is that the order of parameters does not conform to the code style for Encodo.
null
as the first parameter looks strange. The command
can be null, so it should move after the two non-nullable parameters. If we move it all the way to the end, we can even make it optional.hints
should be third.)expressions
not the context
. The first parameter should be the target of the method; the rest of the parameters provide context for that input.params IExpression[]
. Using params
allows a caller to provide zero or more expressions, but it's only allowed on the terminal parameter. Instead, we'll accept an IEnumerable<IExpression>
, which is more standard for the Quino library anyway.The final method signature is below.
bool TryGetExpressionPayload(
[NotNull] IEnumerable<IExpression> expressions,
[NotNull] IExpressionContext context,
CommandFormatHints hints,
out object payload,
[CanBeNull] TCommand command = default(TCommand)
);
The schema migration called the original API like this:
result = Builder.GetExpressionPayload(
null,
CommandFormatHints.DefaultValue,
new ExpressionContext(prop),
prop.DefaultValueGenerator
);
return true;
The call with the new API -- and with the bug fixed -- is shown below. The only non-functional addition is that we have to call ToSequence()
on the first parameter (highlighted). Happily, though, we've fixed the bug and only include a default value in the field definition if one can actually be calculated.
object payload;
if (Builder.TryGetExpressionPayload(
prop.DefaultValueGenerator.ToSequence(),
new ExpressionContext(prop),
CommandFormatHints.DefaultValue,
out payload)
)
{
result = payload as string ?? payload.ToString();
return true;
}
A good rule of thumb is that if you find yourself explaining something in detail, it might still be too complicated. In that light, the call to ToSequence()
is a little distracting.7 It would be nice to be able to map a single expression without having to pack it into a sequence.
So we have one more design decision to make: where do we add that method call? Directly to the interface, right? But the method for a single expression can easily be expressed in terms of the method we already have (as we saw above). It would be a shame if every implementor of the interface was forced to produce this boilerplate.
Since we're using C#, we can instead extend the interface with a static method, as shown below (again, with more line breaks for this article):
public static bool TryGetExpressionPayload<TCommand>(
[NotNull] this IDataCommandBuilder<TCommand> builder, // Extend the builder
[NotNull] IExpression expression,
[NotNull] IExpressionContext context,
CommandFormatHints hints,
out object payload,
[CanBeNull] TCommand command = default(TCommand)
)
{
return builder.TryGetExpressionPayload(
expression.ToSequence(),
context,
hints,
out payload,
command
);
}
We not only avoided cluttering the interface with another method, but now a caller with a single expression doesn't have to create a sequence for it8, as shown in the final version of the call below.
object payload;
if (Builder.TryGetExpressionPayload(
prop.DefaultValueGenerator,
new ExpressionContext(prop),
CommandFormatHints.DefaultValue,
out payload)
)
{
result = payload as string ?? payload.ToString();
return true;
}
We saw in this post how we always have our designer/architect cap on, even when only fixing bugs. We took a look at a quick-fix and then backed out and realized that we were designing a new solution. Then we covered, in nigh-excruciating detail, our thought process as we came up with a new solution.
Many thanks to Dani for the original design and Sebastian for the review!
This is a bit of a riff on ABC -- Always Be Closing -- as popularized by Alec Baldwin in the movie Glengarry Glen Ross.↩
Also, understand that it took much longer to write this blog post and itemize each individual step of how we thought about the issue. In reality, we took only a couple of minutes to work through this chain of reasoning and come up with the solution we wanted. It was only after we'd finished designing that I realized that this was a good example of ABD.↩
Actually, our first instinct is to make sure that there is a failing test for this bug. But, this article deals with how to analyze problems and design fixes, not how to make sure that the code you write is tested. That's super-important, too, though, just so you know. Essential, even.↩
Even though C# doesn't include the exceptions thrown in the signature of a method, as Java does. Where the Java version is fraught with issues, see the "Recoverable Errors: Type-Directed Exceptions" chapter of Midori: The Error Model by Joe Duffy for a really nice proposal/implementation of a language feature that includes expected exceptions in the signature of a method.↩
Which is why we defined the extension method in the first place.↩
I'm fully aware that my name isn't Dave. It's just what ReSharper calls me. Old-school reference.↩
This was pointed out, by the way, by a reviewer of this blog post and escaped the notice of both designers and the code-reviewer. API design is neither easy nor is it done on the first try. It's only finished after multiple developers have tried it out. Then, you'll probably be able to live with it.↩
Most developers would have used new [] { expression }
, which I think is kind of ugly.↩
On Wednesday, Encodo had its first networking event of the year. Our very own Sebastian Greulach presented Code Review Best Practices. A bunch of our friends and colleagues from the area showed up for a lively discussion that, together with the presentation, lasted over 90 minutes.
We heard from people working with remote teams -- off- and near-shored -- as well as people working locally in both small and large teams and for small to large companies. We discussed various review styles, from formal to informal to nonexistent as well as the differences in managing and reviewing code for projects versus products. Naturally, we also covered tool support and where automation makes sense and where face-to-face human interaction is still better.
The discussion continued over a nice meal prepared on our outdoor grill. We even had a lot more vegetables this time! Thanks to lovely weather, we were able to spend some time outside and Pascal demonstrated his L337 drone-flying skills -- but even he couldn't save it from a rain gutter when a propeller came off mid-flight.
Thanks to everyone who helped make it happen and thanks to everyone who showed up!
This first-ever Voxxed Zürich was hosted at the cinema in the SihlCity shopping center in Zürich on March 3rd. All presentations were in English. The conference was relatively small -- 333 participants -- and largely vendor-free. The overal technical level of the presentations and participants was quite high. I had a really nice time and enjoyed a lot of the presentations.
There was a nice common thread running through all of the presentations, starting with the Keynote. There's a focus on performance and reliability through immutabiliy, sequences, events, actors, delayed execution (lambdas, which are relatively new to Java), instances in the cloud, etc. It sounds very BUZZWORDY, but instead it came as a very technically polished conference that reminded me of how many good developers there are trying to do the right thing. Looking forward to next year; hopefully Encodo can submit a presentation.
You can take a look at the VoxxedDays Zürich -- Schedule. The talks that I visited are included below, with links to the presentation page, the video on YouTube and my notes and impressions. YMMV.
Life beyond the Illusion of the Present -- Jonas Bonér
Kotlin - Ready for production -- Hadi Hariri
Used at JetBrains, open-source. 14k+ users. It's not a ground-breaking language. They tried Scala and Scala was the first language they tried to use (Java already being off the table) but they didn't like it, so they invented Kotlin.
Interoperable with Java (of course). Usable from all sorts of systems, but intelliJ Idea has first-class support.
Much less code, less maintenance. Encapsulates some concepts like "data classes" which do what they're supposed for DTO definitions.
JavaScript target exists and is the focus of work. Replacement for TypeScript?
Reactive Apps with Akka and AngularJS -- Heiko Seeberger
During his talk, he took us through the following stages of building a scalable, resilient actor-based application with Akka.
AKKA Distributed Data
AKKA Cluster Sharding
AKKA Persistence
Akka looks pretty good. It guarantees the ordering because ACTORS. Any given actor only exists on any shard once. If a shard goes down, the actor is recreated on a different shard, and filled with information from the persistent store to "recreate" the state of that actor.
DDD (Domain-Driven Design) and the actor model. Watch Hewitt, Meijer and Szyperski: The Actor Model (everything you wanted to know, but were afraid to ask).
Code is on GitHub: seeberger/reactive_flows
Lambda core - hardcore -- Jarek Ratajski
Focus on immutability and no side-effects. Enforced by the lambda calculus. Pretty low-level talk about lambda calculus. Interesting, but not applicable. He admitted as much at the top of the talk.
Links:
expect("poo").length.toBe(1) -- Philip Hofstetter1
This was a talk about expectations of the length of a character. The presenter was very passionate about his talk and went into an incredible amount of detail.
How usability fits in UX - it's no PICNIC -- Myriam Jessier
What should a UI be?
Also nice to have:
Book recommendation: Don't make me think by Steve Krug
Guidelines:
Guidelines for mobile:
Make sure it works on all phones
Give incentives for sharing and purpose (engagement rates make marketing happy. CLICK THE BUTTON)
Keep usability and conversion in mind (not necessarily money, but you actually want people to be using your app correctly)
Usability (can you use your app on the lowest screen-brightness?)
...and more...
Make it pretty (some people don't care, e.g. She very clearly said that she's not aesthetically driven, it's not her field; other people do care. A lot).
Give all the information a customer needs to purchase
Design for quick movement (no lag)
Do usability testing through video
Leverage expectations. Fit in to the environment. Search is on the left? Behind a button? Do that. Don't make a new way of searching.
If you offer a choice, then make them as mutually exclusive as possible. When a company talks to itself (e.g. industry jargon), then users get confused
The registration process should be commensurate to the thing that you're registering for
Small clickable ads on mobile. Make click targets appropriate.
Don't blame negative feedback on "fear of change". It's probably you. If people don't like it, then it might not be user-friendly. The example with Twitter's star vs. heart. It's interesting how we let the world frame our interactions. Why not both? Too complex? Would people really be confused by two buttons? One to "like" and one for "read later"?
Suggested usability testing tools:
React - A trip to Russia isn't all it seems -- Josh Sephton[^3]
This talk was about Web UI frameworks and how his team settled on React.
The reactor programming model for composable distributed computing -- Aleksandar Prokopec[^4]
I am aware of the irony that the emoji symbol for "poo" is not supported on this blogging software. That was basically the point of the presentation -- that encoding support is difficult to get right. There's an issue for it: Add support for UTF8 as the default encoding.↩
In my near-constant striving to be the worst conversational partner ever, I once gave a similar encoding lesson to my wife on a two-hour walk around a lake when she dared ask why mails sometimes have those "stupid characters" in them.↩