Conversation
*add lambda with body feature *add overloads of lambda creation
*add testing *add javadoc documentation
|
This PR issues changes such as: CodeBlock & MethodSpec: A plugin addition to the build, used for producing javadoc from source. |
| checkArgument(permits != null, "permits == null"); | ||
| for (TypeName permit : permits) { | ||
| addPermits(permits); | ||
| addPermits(permit); |
There was a problem hiding this comment.
nit: I'd put this in a separate commit as it's not related to this commit
There was a problem hiding this comment.
Must have missed it while squashing my commits, sorry for the inconvenience. Thanks!
There was a problem hiding this comment.
Have you pushed latest? I'm still seeing this in the 1 commit.
|
Thanks - I'll review as I have time. I'm not very familiar with the JavaPoet code base but I'll do my best. Maybe we can get some others to review from the community. |
| <build> | ||
| <pluginManagement> | ||
| <plugins> | ||
| <plugin> |
There was a problem hiding this comment.
Why is this needed? If just personal preference please put in a separate commit with its own comment.
There was a problem hiding this comment.
I will make sure that every change apart from the lambdas will be in a separate commit
| * @param args the values that should be placed in the holders | ||
| * of the format. | ||
| */ | ||
| public Builder addLambda(List<ParameterSpec> parameters, String expressionFromat, Object... args) { |
There was a problem hiding this comment.
Spelling: expressionFromat should be expressionFormat (multiple places)
| // the inputs of the lambda (left side) | ||
| String inputSide = String.join( | ||
| ", ", parameters.stream() | ||
| .map(p -> p.toString()) |
There was a problem hiding this comment.
Can be: .map(ParameterSpec::toString)
There was a problem hiding this comment.
Whole thing can be:
| .map(p -> p.toString()) | |
| String inputSide = parameters.stream() | |
| .map(ParameterSpec::toString) | |
| .collect(Collectors.joining(", ")); |
There was a problem hiding this comment.
Ah, i see. Thank you for pointing it out!
| // check that the input types are valid | ||
| for (ParameterSpec parameter : parameters) { | ||
| checkArgument(!parameter.type.equals(TypeName.VOID), | ||
| "lambda input parameters cannot be of void type!"); |
There was a problem hiding this comment.
nit: the rest of JavaPoet messages do not use !
| * @param parameters the input parameters of the function. | ||
| * @param body the body of the function. | ||
| */ | ||
| public Builder addLambda(List<ParameterSpec> parameters, CodeBlock body) { |
There was a problem hiding this comment.
To be consistent with the rest of JavaPoet these should be Iterable<ParameterSpec>. You can convert to a stream via:
Stream<ParameterSpec> parameterSpecStream = StreamSupport.stream(parameters.spliterator(), false);
| * @param args the values that should be placed in the holders | ||
| * of the format. | ||
| */ | ||
| public Builder addLambda(List<ParameterSpec> parameters, String expressionFromat, Object... args) { |
There was a problem hiding this comment.
Same as above, make Iterable to match rest of the library
|
I have a general comment about this. I think we should consider a more complete solution. Right now, this modifies CodeBlock and generates a lambda like this: This is good but not ideal. If I were to write the above lambda, I'd write: Instead of modifying CodeBlock with addLambda we probably need a wdyt? |
You are right. My initial thought was to follow the LambdaSpec class idea, and that would be required if we want to allow code like The reason i avoided the idea was because it appeared to me that maybe it wouldnt fit with the rest of the Spec's of the project and was an an overkill to have a separate class for that, thats why i went through with the current implementation. That doesnt mean though that LambdaSpec cant be a thing, and since you propose it as well, i can make the changes needed to support it. My idea of the final code will be something like this: that will output something like this: or to produce: for more general use (for example if we need to pass it as input to a method). We would then have methods such as addLambda(LambdaSpec) in CodeBlock, that would be used for the final construction. Does that make sense? |
|
@HliasMpGH I don't think a |
So you are thinking of a LambdaSpec class that does not follow the builder pattern at all? If its convenient, can you provide me with an example of the idea you propose? Thanks |
|
Sorry, no, I meant keep what you have but make it be able to build an ideal statement. I'm not sure if it's possible. |
Ohh i see. Okay, i will do my best to try and add that ability. |
|
Closing to reduce the pollution made from this PR. The implemented result can be seen (more clearly) in #8 |
*testing
*documentation