Open asjp1970 opened 3 years ago
It sounds like these aren't really Optional parameters at all, since they would never be missing on the built object, so a proper solution wouldn't mark them as Optional (and hence the checkState would fail in your example build method).
I feel like a closer match to your needs would be either:
(Which would be better depends on whether you want users to have the ability to override these properties or not.)
You could approximate the second by implementing the derived getters yourself, fetching from fields that you construct in a post-build method:
@FreeBuilder
abstract class MyType {
private String foo;
public abstract int bar();
private void postBuild() {
foo = "bar=" + bar;
}
public String foo() {
return foo;
}
public class Builder extends MyType_Builder {
public MyType build() {
MyType value = super.build();
value.postBuild();
return value;
}
}
The only problem here is the fields aren't final, which leads to unfortunate concurrency side-effects. Supporting this properly in FreeBuilder would be better.
I can't see a clean way to approximate the first in a way that interacts properly with toBuilder
/mergeFrom
— but I can't see a clean way to implement it under the hood either! Since there's no way to know after calling build
whether the object took the default value or not, there's no way to avoid copying it verbatim to other objects, even if the fields it was derived from change.
Which of these two options most closely matches what you want (if either)? And if it's the default value one, how would you expect something like myObject.toBuilder().bar(10).build()
would affect fields that were originally derived from bar
?
Yeap, the fields not being final is an issue.
In my case the default option is not feasible. And also has the drawback of having to partially re-build all the other properties affected by myObject.toBuilder().bar(10).build()
, as you pointed out.
You are totally right that the other fields are not Optional: they must be part of the build, but we wanted to avoid clients depending from all the JCA class jargon.
Anyway, thanks for your time and I think you can close this issue; I just was after sharing the idea with the expert.
I'd like to leave this open as a potential enhancement for future work. It's unlikely to get worked on soon, but if you felt able to do this yourself, we welcome contributions.
One way of achieving this with decent feature karma, that might unlock other use-cases, would be to allow the abstract base class to declare parameters on its constructor that line up with the getter fields. Then it could create whatever final fields it wanted.
@FreeBuilder
abstract class MyType {
private final String foo;
public abstract int bar();
MyType(int bar) {
foo = "bar=" + bar;
}
public String foo() {
return foo;
}
public class Builder extends MyType_Builder { }
}
This wouldn't work nicely with partials, though.
Alternatively, we could provide a more targeted "derive field X" method:
@FreeBuilder
interface MyType {
String foo();
int bar();
public class Builder extends MyType_Builder {
String foo() {
// The presence of this method would suppress FreeBuilder's normal property
// generation for the foo field.
// The generated code that calls this would have to catch IllegalStateExceptions
// in buildPartial, and turn them into UnsupportedOperationExceptions when foo()
// is called
return "bar=" + bar();
}
}
}
Not sure if it's best to treat "I made a package-protected getter" as code for "this is a derived property", or have some other convention. I have some concerns about dependencies between these fields that might be better solved with one approach or another. For instance, the convention could be "declare a static buildFoo method that takes in all the required properties as parameters", which would give FreeBuilder an explicit dependency graph to let it figure out which methods it needs to call first — and which ones it can't satisfy, in the case of partials, rather than using try/catch.
@FreeBuilder
interface MyType {
String foo();
int bar();
public class Builder extends MyType_Builder {
static String deriveFoo(int bar) {
// The presence of this method would suppress FreeBuilder's normal property
// generation for the foo field.
return "bar=" + bar;
}
}
}
Using a magic name like this has a small risk of accidentally conflicting with existing code, so we might consider an explicit new annotation for such methods (which would have the advantage of being more self-documenting).
Interested to hear any other suggestions.
Drive-by noting another syntax option that occurred to me:
@FreeBuilder
interface MyType {
int bar()
@Cached(lazy=false) default String foo() {
return "bar=" + bar();
}
public class Builder extends MyType_Builder {}
}
The implementation can call the default method once and cache the result (either eagerly or lazily). This avoids magic methods on the Builder, but (for the eager implementation) would need try/catch on Partials. One possible downside is that non-FreeBuilder implementations of this interface are not constrained to implement the caching, and no compile errors will occur if they do not.
I thought of using the Builder pattern to create a rather complex Object containing a chain of certificates and keys. Our intention is to take from clients of the class the burden to provide specific cryptographic parameters; those parameters (algorithms, sizes of keys, salts, etc.) are always fixed and we want everything set under the hood.
To initialize my object I just need a list of parameters, more or less long, but simple: expiration dates, passwords to do PBEncryption, etc. JCA specific attributes (like private keys and X509 certs) can perfectly remain Optional, since I will provide them for clients using the handful of the other (required parameters).
The problem I've found with FreeBuilder is that I cannot hook in the build() method to build the other (complicated) stuff on the client's behalf. Surely is not a problem, and if it is a conscious decision, please disregard the rest of this idea.
Something like this:
In the generated Builder:
And in the inner Builder:
The reasons why I think this could be useful are: