Privatize circle radius and add a constructor/getter

[therealprof]

Signed-off-by: Daniel Egger [email protected]

[hannobraun]

Thank you for the pull request, @therealprof!

What is the justification for making radius private? In the absence of specific reasons, I think that public fields should be preferred, for the following reasons:

• It's less code/complexity.
• You can use destructuring assignment and pattern matching.
• You can use struct update syntax.

I previously merged similar pull requests for types in the math module. I think those made sense, because those types benefited from validation on construction (which I believe has been implemented for them, since then). I don't think the same holds true for fj::Circle and other types from fj.

I believe that any validation of fj types should happen on the kernel side. Primarily, because we might get parallel implementations of fj in other languages at some point, and duplicated code should be kept to a minimum.

[therealprof]

The reason (and this should be obvious from the other PRs) is that requiring to construct a specific object directly directly ties the model code to a specific version and requires to provide all information, even if it could/should be defaulted thus preventing further extension without breaking all existing models.

The color code specifically requires that.

I was also thinking that in the future we might want to accept/use more specific types for some arguments which allow for validation (where it makes sense) and provide automatic type conversion so people don't have to use unwieldy fixed primitive data types like f64 everywhere if they don't want to.

[therealprof]

One thing about the circles which is spefically iffy (and probably shouldn't be allowed!) is the possible use of negative radiuses. While this seems to render okay by itself, this will create objects with a negative bounding box which is not okay.

And to add insult to the injury, look what happens if we sweep the spacer using negative radiuses by a negative height:

[hannobraun]

The reason (and this should be obvious from the other PRs)

Well, I'm not as smart as you, so I appreciate that you spelled it out 😁

is that requiring to construct a specific object directly directly ties the model code to a specific version and requires to provide all information, even if it could/should be defaulted thus preventing further extension without breaking all existing models.

The color code specifically requires that.

Okay, I'll allow it. I have some reservation about the way the color code is done, but those are not critical, and I'll add a comment in the respective PR.

I was also thinking that in the future we might want to accept/use more specific types for some arguments which allow for validation (where it makes sense) and provide automatic type conversion so people don't have to use unwieldy fixed primitive data types like f64 everywhere if they don't want to.

One thing about the circles which is spefically iffy (and probably shouldn't be allowed!) is the possible use of negative radiuses. While this seems to render okay by itself, this will create objects with a negative bounding box which is not okay.

Yeah, that makes sense.

And to add insult to the injury, look what happens if we sweep the spacer using negative radiuses by a negative height:

That's funny 😄

The code that determines how many vertices are needed to make the circle round enough to satisfy the tolerance value probably gets confused by the negative radius.

[hannobraun]

Closing in favor of #343, which contains the commit from this pull request.