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

Tobias Bocanegra commented on JCRVLT-335:
-----------------------------------------

you are correct. thanks for pointing this out. see my proposed fix: 
https://github.com/apache/jackrabbit-filevault/pull/44/files

It adds a new interface `WalkableAggregate` to make it more neat.

> Remove AggregateImpl cast in DocViewSerializer 
> -----------------------------------------------
>
>                 Key: JCRVLT-335
>                 URL: https://issues.apache.org/jira/browse/JCRVLT-335
>             Project: Jackrabbit FileVault
>          Issue Type: Improvement
>          Components: vlt
>            Reporter: Radosław Wesołowski
>            Assignee: Tobias Bocanegra
>            Priority: Major
>             Fix For: 3.2.6
>
>
>  The 
> [cast|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSerializer.java#L48]
>  of 
> [Aggregate|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/Aggregate.java]
>  to 
> [AggregateImpl|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java]
>  in 
> [DocViewSerializer|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSerializer.java]'s
>  constructor as in
> {code:java}
> public DocViewSerializer(Aggregate aggregate) {
>       this.aggregate = (AggregateImpl) aggregate;
> }
> {code}
> is breaking at least the [Liskov substitution 
> principle|https://en.wikipedia.org/wiki/Liskov_substitution_principle]. Any 
> client of such class would be surprised to see it break upon passing a 
> seemingly correct parameter of type 
> [Aggregate|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/Aggregate.java].
>  The least that could be done would be to replace the parameter type to 
> [AggregateImpl|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java].
>  In the current form the design has a definitive code smell unfortunately.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to