[ 
https://issues.apache.org/jira/browse/CASSANDRA-20404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17944886#comment-17944886
 ] 

Maulin Vasavada commented on CASSANDRA-20404:
---------------------------------------------

[~smiklosovic] I wonder what changed between before and now that results in so 
many failures? Before the latest refactoring we had the build() method call 
applyConfig() AND even prior to that every 'with' method was calling 
applyConfig() also. Tests should have the same issue before also.

> Improve EncryptionOptions class's `with` methods using Builder pattern
> ----------------------------------------------------------------------
>
>                 Key: CASSANDRA-20404
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-20404
>             Project: Apache Cassandra
>          Issue Type: Improvement
>          Components: Legacy/Core
>            Reporter: Maulin Vasavada
>            Assignee: Maulin Vasavada
>            Priority: Normal
>             Fix For: 5.x
>
>          Time Spent: 2h 50m
>  Remaining Estimate: 0h
>
> Currently, 
> [EncryptionOptions.java|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/config/EncryptionOptions.java#L520]
>  allows constructing the object using `with` prefix method pattern which 
> sounds similar to the Builder Pattern 
> ([example)|https://blogs.oracle.com/javamagazine/post/exploring-joshua-blochs-builder-design-pattern-in-java]
>  However, when there is any new parameter introduced in the class for a 
> corresponding configuration option, it requires changes to all the 
> constructor calls from those `with` methods to reflect the new parameter. 
> This is error prone given so many string parameters and considering some 
> [special 
> use-cases|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/config/EncryptionOptions.java#L1190]
>  it makes it challenging to modify the code for a simple task - adding a new 
> parameter in the class.
> Proposal here is to use the Builder pattern (like shows in the example linked 
> above) to improve the code in EncryptionOptions.java file.
> There could be a potential to consider using "java record" type that got 
> introduced in Java 14+ or using [Lombok plugin|https://projectlombok.org/] 
> but in order to be backward compatible and avoid additional dependency on 
> external packages- may be we can simply follow the builder pattern.
> One change that we have to be careful about is - the new builder pattern 
> implementation may not call `applyConfig()` everytime upon calling `with` 
> methods compared to the current implementation. Only when final 
> `Builder#build()` happens it will call the applyConfig() method . While that 
> saves multiple initialization that happen today in Tests due to heavy usage, 
> we have to assess if there is any impact due to this behavior change.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to