search

oasp / oasp4j

The Open Application Standard Platform for Java
Apache License 2.0
60 stars 303 forks source link

Add prefix 'Abstract' in the name for all the relevant abstract classes #576

Closed kiran-vadla closed 7 years ago

kiran-vadla commented 7 years ago

Fix for Issue #561 is provided as part of this PR . As a fix . added prefix 'Abstract' in the name for all the relevant abstract classes.

As part of the fix , did not add the prefix to the classes 'PermisssionConstants.java' and 'NamedQueries.java' . Reasons were two-fold : First being , that these are classes where constants are defined . Ideally it should not have been defined 'abstract' . Second reason being , adding the prefix 'Abstract' in the name will have effect on the CobiGen code since the names of these classes are referenced from Cobigen Code and also JUNITs were failing. Apart from these two java files , there were few more framework classes that were referenced from Cobigen Code . Hence this change is not applied to these framework classes as well so as to prevent Cobigen from breaking.

There were couple of abstract classes namely 'BaseTest' and 'BaseWebSecurityConfig' . As part of the fix , these classes are renamed to AbstractTest and AbstractWebSecurityConfig respectively.

Thanks, @kiran-vadla

oasp-ci commented 7 years ago

Can one of the admins verify this patch?

amarinso commented 7 years ago

I see two things wrong here:

Take for example ProductEto. It is a class pertaining to the domain model of the application, on the logic layer and it should be possible to instantiate it directly (via new ProductEto) if required... so it can not be abstract.

What was the criteria to consider these (and other) classes as candidates for refactoring?. Is that they extend AbstractEto (and AbstractTo) ? In that case these other base classes are ok being abstract because they have no sense on their own... they don't pertain to any application domain.

kiran-vadla commented 7 years ago

Hi @amarinso , I have submitted the changes for the review comments . Please review .

kiran-vadla commented 7 years ago

Since the changes as part of the review comments of this PR are many , fresh PR #585 is raised . Hence this will be closed .