I encountered some curious behavior while writing a service-locator interface (protocol) in Swift. I've reproduced the issue in a stripped-down playground¹ and am almost certain I've found a bug in the Swift 3.0.1 compiler included in XCode 8.2.1.

A Simple, Generic Function

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.

Or Maybe Not So Simple?

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 expected
• odd: 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.² I would have expected the type to be ((Int, Int)) instead.
• complexTuple: As with tuple, the compiler interprets the type for this call correctly.

Narrowing Down the Issue

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 expected
• odd: 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.

Swift Grammar

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 tuple³
• (1) => Tuple<int>
• ((1)) => Tuple<Tuple<int>>
• ...and so on.

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.

Conclusion

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)))

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).

Quino: from 1.13 to 4.x

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.

For over a year, I've been writing articles about various parts of Quino. A customer asked if I could collect those links into a coherent table of contents, to make it easier to use as a reference.

Configuration

• Encodos configuration library for Quino -- Part 1
• Encodos configuration library for Quino -- Part 2
• Encodos configuration library for Quino -- Part 3
• Starting up an application, in detail
• Mini-applications and utilities with Quino

Modeling

A scalable pattern for building metadata

Data Driver

• Quino Data Driver architecture, Part I: Applications & Sessions
• Quino Data Driver architecture, Part II: Command types & inputs
• Quino Data Driver architecture, Part III: The Pipeline
• Schema migration in Quino 1.13
• An introduction to query-mapping in the ORM

Data Optimization

• Optimizing data access for high-latency networks: part I
• Optimizing data access for high-latency networks: part II
• Optimizing data access for high-latency networks: part III
• Optimizing data access for high-latency networks: part IV
• Quino: efficiency, hinting and local sorting
• Quino: partially-mapped queries
• Running Quino applications with a local database

Custom SQL

• Mixing your own SQL into Quino queries: part 1 of 2
• Mixing your own SQL into Quino queries: part 2 of 2

API design

• API Design: Running an Application -- Part 1
• API Design: To Generic or not Generic? -- Part 2

Check out two new talks on our web site:

• TechTalk: .NET Standard 2.0
• Die Wahrheit über Code Reviews So klappt's!

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!

The summary below describes major new features, items of note and breaking changes. The full list of issues is also available for those with access to the Encodo issue tracker.

Highlights

This release is a "bridge" release that has the entire new Metadata API as well as the older version, which is marked as obsolete. It is intended that projects upgrade to this version only temporarily in order to more easily migrate to the 4.0 Metadata API. At that point, projects should immediately upgrade to Quino 4.0, from which all obsolete methods have been removed. Once 4.0 is available, there will be no more bug-fix releases for this release.

Metadata construction

• Remove MetaId/Guid parameters from all metadata-building APIs
• Remove all dependencies and requirement for the MetaBuilder; make MetaBuilder obsolete.
• Drastically reduce the surface of the MetadataBuilder and base classes and improve dependency-resolution
• Replaced AddClassWithDefaultPrimaryKey("A") with AddClass("A").AddId()
• Added AddForeignKeys() step to builders and AddForeignKey() APIs
• Moved a lot of the builder-related extension methods from Quino.Meta to Quino.Builders.
• Added API to make it easier to add paths: AddPath(classA.FromOne(), classTwo.ToMany("OneId"))
• Fixed generation of extension classes (e.g. PunchclockUser).

Data & Schema

• Made all GenericObject constructors without an IDataSession obsolete.
• Removed last reference to the GlobalContext from the data driver
• Improved flexibility of caching subsystem and added default global/immutable-data cache (in addition to the session-based cache).
• Fixed schema-migration for SQL Server: not-null constraints are now properly added and trigger-migration works with pure SQL statements and has been drastically simplified.

Configuration

• Added BootstrapServices to the application to clearly delineate which objects are needed during the boostrap phase of configuration and startup
• Re-added all satellite assemblies for all Quino Nuget packages (ch-DE translations)

GUI

• Replaced all usages of DevExpress TreeList with GridControl/GridView.
• Added a lot of WPF controls and application implementation with a Sandbox.WPF project

Breaking changes

• PersistentEventHandlerAspect has been renamed to PersistentEventHandlerAspectBase

Roadmap

The next step is to bring out the 4.0 release, which will include the following features,

• Remove all currently obsolete code
• Reshape the metadata API to be more task-based and to significantly reduce the surface revealed to the application developer. This includes a drastic reduction of extension methods in Quino.Meta
• Finish implementation and support for layouts everywhere (specifically as they are used for titles).
• Split Quino.Data.Backend out of Quino.Data to reduce the surface exposed to application developers.
• Deprecate IMetaReadable, IMetaWritable and IPersistable and replace with a more appropriate and slimmer API.

From a customer, we got the request to apply a visual style guide (VSG) to a Bootstrap-based application. Since we do have a lot of experience with applying style guides on web applications and styling in general, we accepted the job and started to evaluate the details.

Which version of Bootstrap to use

The most recent stable version of Bootstrap is 3.3.6. However, when you go to the Bootstrap website, there is an announcement that Bootstrap 4 "is coming". The current state of Bootstrap 4 is alpha and the last blog post is from December 2015 which is almost half a year ago. It also is not clear, when version 4 finally will be available and stable and so we had to use the old Bootstrap 3 for this project.

But even here, there is some obscurity going on: Bootstrap was initially developed with LESS but for some reason they decided to switch to SASS. Even if we prefer to use LESS at Encodo, we decided to use SASS for this project to be able to upgrade to Bootstrap 4 more easily when it's available. There is also a SASS version of Bootstrap available which we decided to use as the base for this project.

How to customize Bootstrap

Bootstrap is a GUI library that is intended to be as simple as possible to use for the consuming developer. Unfortunately, this does not mean that it is also simple to create a theme for it or to modify some of the existing components.

There is a customization section on the Bootstrap website that allows you to select the needed components and change some basic thing like colors and a few other options. This might be very nice if you just want to use Bootstrap with your own colors but since we had a style guide with a layout quite different from Bootstrap, we could not use this option.

So we decided to clone the entire Bootstrap library, make our changes and then build our custom Bootstrap version. This makes it possible to add some custom components and change the appearance of existing elements.

Problems we ran into

Bootstrap provides support for all kinds of browsers, including Internet Explorer down to version 8. While this is nice for developing an application that runs anywhere, it makes the SASS styling code very hard to read and edit. Also, you cannot use modern technologies such as Flexbox that makes the styling a lot easier and is the base of every layout we have created in the recent past.

Another important point is that the modularity of components is not really given. For example, the styles for the button are defined in one file, but there are many other locations where you can find styles for buttons and that modify the appearance of the button based on the container.

Also, the styles are defined "inside-out" which means that the size of a container is defined by its content. Styleguides normally work the other way around. All these points make it hard to change the structure of the page without affecting everything else. Especially when you try to use the original Bootstrap HTML markup that may not match the needs of the desired layout.

To increase the struggles, there is also the complex build- and documentation system used in the Bootstrap project. It might be great that Bootstrap itself is used for the documentation but I cannot understand why there is another CSS file with 1600 lines of code that changes some things especially for the documentation. Of course this messes up our painstakingly crafted Bootstrap styles again. In the end, we had to remove this file from our demo site, which broke styling for some documentation-specific features (like the sidebar menu).

Another point of concern is that Bootstrap uses jQuery plugins for controls that require JavaScript interaction. This might be good for simple websites that just need some basic interaction but is counterproductive for real web applications because the jQuery event handling can interfere with web application frameworks such as React or Angular.

When to use bootstrap

I do not think that Bootstrap is a bad library but it is not really suitable for projects like this. The main use case of Bootstrap is to provide a good-looking layout for a website with little effort and little foreknowledge required. If you just want to put some information in the web and do not really care how it looks as long as it looks good, then Bootstrap is a good option for you.

If you'd like more information about this, then please feel free to contact us!

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 topic¹, led me to ask the developers at Encodo what they thought.

• Urs was open to the idea of using tabs because then "everyone can use whatever he likes and we can avoid the unnecessary discussion about the ideal value (why does it have to be an even value? I want 3 spaces!!!) Further, we might be able to save some disk space ;)"
• Sebastian was emphatically not open to the idea of tabs because "Tabs is just a lie. There are never only tabs. I've seen multiple projects with tabs, there are always spaces as well and this breaks the formatting depending on your settings."
• Wadim pointed out that "the tab key produces a character that is used for indentation" -- heavily hinting that people who use spaces are doing it wrong -- and then backed up Urs by suggesting 3 spaces per tab.
• Fabi cited Death to the Space Infidels! by Jeff Atwood, "What does matter is that you, and everyone else on your team, sticks with those conventions and uses them consistently," then expressed a preference for two spaces, but agreeing that four might be easier since that's the standard used by other companies.
• Remo backed up Sebastian in saying that tabs are bad, writing that "I have worked on projects where we tried to use tabs. But this always ended up in chaos somehow." Two or four is fine -- the longer you work with one, the odder the other one looks. "Personally I think using 2 or 4 spaces takes some time getting used to it. After that, both are well suited to read code with a slight advantage for 4 spaces because the "column" widths are wider and it's therefore easier to find the closing braces when scanning vertically (our screens are really wide - so the loss of valuable space is no longer an argument)."
• Pascal was along the same lines as Fabi. He made a good point for spaces, writing "I personally prefer spaces since it takes the whole configuration in all the tools out of the picture at once."
• Robin also pleaded for consistency above all, writing "I like tabs more" and "I'm used to a width of 2 spaces".
• Marco see the advantage of tabs for customization, but understands that it will probably lead to time wasted converting whitespace. He's accustomed to 2 spaces and Encodo has a ton of code with two spaces. Although Fabi says he sees a lot of code with four-space indents, Marco's seen a lot of code with two-space indents.

So, with the rewrite of the Encodo C# Handbook in progress, what will be our recommendation going forward?

Let's summarize the opinions above:

• Consistency is paramount (Fabi, Pascal, Robin,...pretty much everyone)
• Using tabs has, in the past, inevitably led to a mix of tabs and spaces (Marco, Sebastian, Remo)
• An indent of 3 spaces would be nice (Urs, Wadim)
• Everyone else likes either a four-space indent or two while others don't really care either way. Nobody wants eight².

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.³

# 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

Overview

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.

Requirements

Before we write any code, we should know what we're doing.¹

• We use aspects (IMetaAspects) in Quino to add domain-specific metadata (e.g. the IVisibleAspect controls element visibility)
• Suppose we have such an aspect with properties A1...AN. When we set property A1 to a new value, we want to retain the values of properties A2...AN (i.e. we don't want to discard previously set values)
• The current pattern is to call 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).
• Any properties on the requested type that you want to change must have setters.
• If the requested type is an interface, then we end up defining our interface as mutable.
• Other than when building the metadata, every other use of these interfaces should not make changes.
• We would like to be able to define the interface as read-only (no setters) and make the implementation mutable (has setters). Code that builds the metadata uses both the interface and the implementation type.

Although we're dealing concretely with aspects in Quino metadata, the pattern and techniques outlined below apply equally well to other, similar domains.

The current API

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?

The new API

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.

Making it work for one aspect type

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.

Generalize the aspect type

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;
}

Remove constructor restriction

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;
}

Just pass in the new aspect to use

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.

Redefine original method

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
  );
}

Generalize copying values

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
  );
}

Final implementation

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.

Conclusion

This is actually pretty neat and clean:

• We have a pattern where all properties on the interface are read-only
• We have a pattern where an aspect can indicate how its values are to be copied from another instance. This is basically boilerplate, but must be written only once per aspect -- and it can be located right in the implementation itself rather than in an extension method.
• A caller building metadata passes in a single lambda to set values. Existing values are handled automatically.
• Adding support for more aspects is straightforward and involves very little boilerplate.

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.¹ 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.²

A bug in the schema migrator

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.

Is there a quick fix?

Our first instinct is, naturally, to try to figure out how we can fix the problem.³ 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.

• Do we throw an exception? No, we can't just do that. None of the callers are expecting an exception, so that's an API change.⁴
• Do we return 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.

Part of the result is missing

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:

• Can the given expressions be mapped to a string (the target representation)?
• If so, what is that text?

Instead of a Get method, this is a classic TryGet method.

How to Introduce the Change

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.

What are the parameters?

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.⁵

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.⁶

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
);

Are We Done?

Not quite. There are two things that are still wrong with this signature, both important.

Fix the Result Type

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.

Fix parameters

Another problem is that the order of parameters does not conform to the code style for Encodo.

• We prefer to place all non-nullable parameters first. Otherwise, a call that passes 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.
• Also, primitives should come after the references. (So hints should be third.)
• Also, semantically, the call is getting the payload for the expressions not the context. The first parameter should be the target of the method; the rest of the parameters provide context for that input.
• The original method accepted 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)
);

Our API in Action

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;
}

One More Design Decision...

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.⁷ 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 it⁸, 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;
}

Conclusion

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!

The article .NET Core, a call to action by Mark Rendle exhorts everyone to "go go go".

I say, "pump the brakes."

RC => Beta => Alpha

Mark says, "The next wave of work must be undertaken by the wider .NET community, both inside and outside Microsoft."

No. The next wave of work must be undertaken by the team building the product. This product is not even Beta yet. They have called the last two releases RC, but they aren't: the API is still changing quite dramatically. For example, the article Announcing .NET Core RC2 and .NET Core SDK Preview 1¹ lists all sorts of changes and the diff of APIs between RC1 and RC2 is gigantic -- the original article states that "[w]e added over a 1000 new APIs in .NET Core RC2".

What?!?!

That is a huge API-surface change between release candidates. That's why I think these designations are largely incorrect. Maybe they just mean, "hey, if y'all can actually work with this puny footprint, then we'll call it a final release. If not, we'll just add a bunch more stuff until y'all can compile again." Then, yeah, I guess each release is a "candidate".

But then they should just release 1.0 because this whole "RC" business is confusing. What they're really releasing are "alpha" builds. The quality is high, maybe even production-quality, but they're still massive changes vis-a-vis previous builds.

An Example: Project files

That doesn't sound like "RC" to me. As an example, look at the project-file format, project.json.

Mark also noted that there are "no project.json files in the repository" for the OData project that comes from Microsoft. That's not too surprising, considering the team behind .NET Core just backed off of the project.json format considerably, as concisely documented in The Future of project.json in ASP.NET Core by Shawn Wildermuth. The executive summary is that they've decided "to phase out project.json in deference to MSBuild". Anyone who's based any of their projects on the already-available-in-VS-2015 project templates that use that format will have to convert them to whatever the final format is.

Wildermuth also wrote that "Microsoft has decided after the RTM of the ASP.NET Core framework to phase out project.json and use MSBuild for build data. (Emphasis added.)" I was confused (again) but am pretty sure that he's wrong about RTM because, just a couple of days later, MS published an article Announcing ASP.NET Core RC2 -- and I'm pretty sure that RCs come before RTM.

Our Experience

At Encodo, we took a shot at porting the base assembly of Quino to .NET Core. It has only dependencies on framework-provided assemblies in the GAC, so that eliminated any issues with third-party support, but it does provide helper methods for AppDomains and Reflection, which made a port to .NET Core nontrivial.

Here's a few things we learned that made the port take much longer than we expected.

• Multi-target project.json works with the command-line tools. Create the project file and compile with dotnet.
• Multi-target project.json do not work in Visual Studio; you have to choose a single target. Otherwise, the same project that just built on the command line barely loads.
• Also, Visual Studio ignores any #IFDEFs you use for platform-specific code. So, even if you've gotten everything compiling on the command-line, be prepared to do it all over again differently if you actually want it to work in VS2015.
• If you do have to change code per-platform (e.g. for framework-only), then you have to put that code in its own assembly if you want to use Visual Studio.
• If you go to all the trouble to change your API surface to accommodate .NET Core, then you might have done the work for nothing: many of the missing APIs that we had to work around in porting Encodo.Core are suddenly back in RC2. That means that if we'd waited, we'd have saved a lot of time and ended up in the same place.
• There are several versions and RCs available, but only the beta channel was usable for us (e.g. the RC3 versions didn't work at all when we tried them).
• In the end, we didn't have to make a lot of changes to get Encodo.Core compiling under .NET Core.
• We learned a lot and know that we won't have too much trouble porting at least some assemblies, but the tools and libraries are still not working together in a helpful way -- and that ends up eating a lot of time and effort.

With so much in flux -- APIs and project format -- we're not ready to invest more time and money in helping MS figure out what the .NET Core target needs. We're going to sit it out until there's an actual RTM. Even at that point, if we make a move, we'll try a small slice of Quino again and see how long it takes. If it's still painful, then we'll wait until the first service pack (as is our usual policy with development tools and libraries).

Conclusion

I understand Mark's argument that "the nature of a package-based ecosystem such as NuGet can mean that Project Z can't be updated until Project Y releases .NET Core packages, and Project Y may be waiting on Project X, and so on". But I just don't, as he says, "trust that what we have now in RC2 is going to remain stable in API terms", so I wouldn't recommend "that OSS project maintainers" do so, either. It's just not ready yet.

If you jump on the .NET Core train now, be prepared to shovel coal. Oh, and you might just have to walk to the next station, too. At noon. Carrying your overseas trunk on your back. Once you get there, though, you might be just in time for the 1.0.1 or 1.0.2 express arriving at the station, where you can get on, you might not even have to buy a new ticket -- and you can get there at the same time as everyone else.