You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Radosław Wesołowski (JIRA)" <ji...@apache.org> on 2019/03/21 15:10:00 UTC

[jira] [Updated] (JCRVLT-335) Remove AggregateImpl cast in DocViewSerializer

     [ https://issues.apache.org/jira/browse/JCRVLT-335?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Radosław Wesołowski updated JCRVLT-335:
---------------------------------------
    Description: 
 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.

 

  was:
 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 [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.

 


> 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
>            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)