Make data classes in db package immutable

[rfc2822]

Currently we have var properties in the database entity data classes.

I think there should be only immutable data classes, at least they're immutable in the documentation.

• Check whether we really need mutable data classes.
• Either document why or make them immutable.

[sunkup]

While it's not shown in the examples, they don't say that it's outright "bad" to use mutable properties for the room data class entities and I think it's quite convenient to be honest. We can make all of them immutable, but we would have to create copies of the original entity objects every time we want to change values. So

  localHomeset.displayName = ...
  localHomeset.privBind =  ...
  homeSetRepository.insertOrUpdateByUrl(localHomeset)

becomes

  val newLocalHomeset = localHomeset.copy(
      displayName =  ...,
      privBind =  ...
  )
  homeSetRepository.insertOrUpdateByUrl(newLocalHomeset)

and I don't really see the benefit in that. In the PR I have made the properties immutable which are not updated.

[rfc2822]

I think it's quite convenient to be honest. We can make all of them immutable, but we would have to create copies of the original entity objects every time we want to change values.

While I agree that it's convenient (which is probably the reason why they are not immutable at the moment), I wonder whether it's a sign of bad design when data transfer objects are used as a place where modifications etc happen.

When we pass for instance a Collection around, immutable objects would prevent methods from changing fields without saving the object. When we have to make a copy, it makes it explicit that this is only a copy, so it must be saved to the database with an extra call.

I found a lot of article about this topic out there, with different conclusions. So it seems like a matter of taste.

Another question, if we'd choose mutability, is: which properties should be mutable and which not? Or probably just var for anything by default?

So I'd go either the one or the other way, but mixing (like in the current codebase, where we have many vars and then for instance some vals in SyncStats) seems strange to me.

So what do you think? Also @ArnyminerZ

[sunkup]

I do not have a strong opinion on this. I like the ease of use with mutable properties, but also see the security/stability immutability brings. I am fine with either.

[rfc2822]

I do not have a strong opinion on this. I like the ease of use with mutable properties, but also see the security/stability immutability brings. I am fine with either.

Ok, so let's make them immutable :)

[ArnyminerZ]

So what do you think? Also @ArnyminerZ

I'd stick with immutable for all data classes. Even though that it's a bit more code, I think it can lead to better practices. Mainly for lists, which I think is what has most issues with Compose.

Another question, if we'd choose mutability, is: which properties should be mutable and which not? Or probably just var for anything by default?

In that regard, since I don't think is ever good to mix var with val, and we really should make some variables val, I'd just stick with everything like that, and use copy whenever is needed.