Skip to content

Refactor Address

Bengfort requested to merge spike-inline-address into master

Back when contacts were added we had quite some discussions about the fact that they were split into 4 models (Contact, Address, City, Country). The discussion is mostly summed up in !50 (comment 926).

Now, with some distance, I wanted to reevaluate the decision.

Complexity

The ContactForm really is a mess. On closer inspection however, I realized that the model split is not the (only) driving factor here. Guardians have a big part to play. Another source for complexity is the fact that address fields are only required if one of them is not empty.

Combining two forms turned out to be less of an issue than I had anticipated. I also think there might be ways to integrate them in an even better way, see 577b2f4a.

Database normalization

The argument goes like this: Everyone who lives in "Berlin" also lives in "Germany", so storing this information on each contact is redundant and can lead to inconsistencies.

Note that following this argument, one could also argue that there should be separate models for Street and House.

I would completely agree with this if "Berlin" in this case was a city. But in my impression it is more "The string we print on a letter". This is admittedly a small difference. Still, we do not use relations between contacts, addresses, cities, and countries anywhere in the software.

Conclusion

The differences turn out to be minor. So here is my proposal:

  • Inline City and Country into Address. Depending on your interpretation, this denormalizes the database, but reduces the amount of boilerplate code significantly.
  • Refactor ContactForm
  • Keep Address as a separate model. This is not really helpful in terms of normalization. But it also doesn't add much complexity and groups some fields in a meaningful way.

Merge request reports