Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generated constructors for Records should not have default arguments #60

Open
marcgrr opened this issue Apr 25, 2017 · 0 comments
Open

Comments

@marcgrr
Copy link
Contributor

marcgrr commented Apr 25, 2017

(I'm talking about the generated scala code, and I've never looked at the code generators for other languages, but I assume that those might have similar behavior.)

Issue description

When you give a field in a courier record a default value, it generates an apply method with default arguments for that field. This can hide bugs where you construct the record without giving it all the data that it needs. Therefore, I suggest that the generated methods not have default arguments.

Obviously, this will probably break a lot of existing code that uses courier. Maybe there should be some sort of "generateDefaultArgument" annotation to ease the transition.

Example

Here is an example of a bug that the current behavior hid from me:

record SpecificationWithId {
  creatorName: CreatorName
  name: SpecificationName
  ...Specification
}
record Specification {
  isStandalone: boolean?
  template: AnyData
  children: array[NodeRequest] = []
  preCreatedNodes: array[PreCreatedNode] = []
}
object SpecificationWithIds {

  def toTuple(specificationWithId: SpecificationWithId):
    (QualifiedSpecificationName, Specification) = {
    val qualifiedSpecificationName = QualifiedSpecificationName(
      specificationWithId.creatorName,
      specificationWithId.name)
    val specification = Specification(
      specificationWithId.isStandalone,
      specificationWithId.template,
      specificationWithId.children)
    (qualifiedSpecificationName, specification)
  }

}

I added the preCreatedNodes field after I wrote the SpecificationWithIds.toTuple "deconstructor", and I forgot that I needed to update the "deconstructor".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant