From 17f106c6eadff21b70f4928c474fca13471afbb1 Mon Sep 17 00:00:00 2001
From: Gunnar Morling <gunnar.morling@googlemail.com>
Date: Sun, 30 Jun 2013 10:56:27 +0200
Subject: [PATCH] #32 Some clean-up

---
 .../processor/MethodRetrievalProcessor.java   | 148 ++++++++++--------
 .../ErronuousCollectionMappingTest.java       |   2 +-
 .../ErronuousMapper.java                      |   2 +-
 3 files changed, 81 insertions(+), 71 deletions(-)
 rename processor/src/test/java/org/mapstruct/ap/test/collection/{erronuous => erroneous}/ErronuousCollectionMappingTest.java (97%)
 rename processor/src/test/java/org/mapstruct/ap/test/collection/{erronuous => erroneous}/ErronuousMapper.java (95%)

diff --git a/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java b/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java
index 09a03783b..76b15bd8c 100644
--- a/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java
+++ b/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java
@@ -89,75 +89,10 @@ public class MethodRetrievalProcessor implements ModelElementProcessor<TypeEleme
 
         MapperPrism mapperPrism = implementationRequired ? MapperPrism.getInstanceOn( element ) : null;
 
-        //TODO Extract to separate method
-        for ( ExecutableElement method : methodsIn( element.getEnclosedElements() ) ) {
-            Parameter parameter = executables.retrieveParameter( method );
-            Type returnType = executables.retrieveReturnType( method );
-
-            boolean mappingErroneous = false;
-
-            if ( implementationRequired ) {
-                if ( parameter.getType().isIterableType() && !returnType.isIterableType() ) {
-                    printMessage(
-                        ReportingPolicy.ERROR,
-                        "Can't generate mapping method from iterable type to non-iterable type.",
-                        method
-                    );
-                    mappingErroneous = true;
-                }
-                if ( !parameter.getType().isIterableType() && returnType.isIterableType() ) {
-                    printMessage(
-                        ReportingPolicy.ERROR,
-                        "Can't generate mapping method from non-iterable type to iterable type.",
-                        method
-                    );
-                    mappingErroneous = true;
-                }
-                if ( parameter.getType().isPrimitive() ) {
-                    printMessage(
-                        ReportingPolicy.ERROR,
-                        "Can't generate mapping method with primitive parameter type.",
-                        method
-                    );
-                    mappingErroneous = true;
-                }
-                if ( returnType.isPrimitive() ) {
-                    printMessage(
-                        ReportingPolicy.ERROR,
-                        "Can't generate mapping method with primitive return type.",
-                        method
-                    );
-                    mappingErroneous = true;
-                }
-
-                if ( mappingErroneous ) {
-                    continue;
-                }
-            }
-
-            //add method with property mappings if an implementation needs to be generated
-            if ( implementationRequired ) {
-                methods.add(
-                    Method.forMethodRequiringImplementation(
-                        method,
-                        parameter.getName(),
-                        parameter.getType(),
-                        returnType,
-                        getMappings( method )
-                    )
-                );
-            }
-            //otherwise add reference to existing mapper method
-            else {
-                methods.add(
-                    Method.forReferencedMethod(
-                        typeUtil.getType( typeUtils.getDeclaredType( element ) ),
-                        method,
-                        parameter.getName(),
-                        parameter.getType(),
-                        returnType
-                    )
-                );
+        for ( ExecutableElement executable : methodsIn( element.getEnclosedElements() ) ) {
+            Method method = getMethod( element, executable, implementationRequired );
+            if ( method != null ) {
+                methods.add( method );
             }
         }
 
@@ -176,6 +111,81 @@ public class MethodRetrievalProcessor implements ModelElementProcessor<TypeEleme
         return methods;
     }
 
+    private Method getMethod(TypeElement element, ExecutableElement method, boolean implementationRequired) {
+        Parameter parameter = executables.retrieveParameter( method );
+        Type returnType = executables.retrieveReturnType( method );
+
+        //add method with property mappings if an implementation needs to be generated
+        if ( implementationRequired ) {
+            boolean isValid = checkParameterAndReturnType( method, parameter.getType(), returnType );
+
+            if ( isValid ) {
+                return
+                    Method.forMethodRequiringImplementation(
+                        method,
+                        parameter.getName(),
+                        parameter.getType(),
+                        returnType,
+                        getMappings( method )
+                    );
+            }
+            else {
+                return null;
+            }
+        }
+        //otherwise add reference to existing mapper method
+        else {
+            return
+                Method.forReferencedMethod(
+                    typeUtil.getType( typeUtils.getDeclaredType( element ) ),
+                    method,
+                    parameter.getName(),
+                    parameter.getType(),
+                    returnType
+                );
+        }
+    }
+
+    private boolean checkParameterAndReturnType(ExecutableElement method, Type parameterType, Type returnType) {
+        if ( parameterType.isIterableType() && !returnType.isIterableType() ) {
+            printMessage(
+                ReportingPolicy.ERROR,
+                "Can't generate mapping method from iterable type to non-iterable type.",
+                method
+            );
+            return false;
+        }
+
+        if ( !parameterType.isIterableType() && returnType.isIterableType() ) {
+            printMessage(
+                ReportingPolicy.ERROR,
+                "Can't generate mapping method from non-iterable type to iterable type.",
+                method
+            );
+            return false;
+        }
+
+        if ( parameterType.isPrimitive() ) {
+            printMessage(
+                ReportingPolicy.ERROR,
+                "Can't generate mapping method with primitive parameter type.",
+                method
+            );
+            return false;
+        }
+
+        if ( returnType.isPrimitive() ) {
+            printMessage(
+                ReportingPolicy.ERROR,
+                "Can't generate mapping method with primitive return type.",
+                method
+            );
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Retrieves the mappings configured via {@code @Mapping} from the given
      * method.
diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/erronuous/ErronuousCollectionMappingTest.java b/processor/src/test/java/org/mapstruct/ap/test/collection/erroneous/ErronuousCollectionMappingTest.java
similarity index 97%
rename from processor/src/test/java/org/mapstruct/ap/test/collection/erronuous/ErronuousCollectionMappingTest.java
rename to processor/src/test/java/org/mapstruct/ap/test/collection/erroneous/ErronuousCollectionMappingTest.java
index ac90935b5..8e061927b 100644
--- a/processor/src/test/java/org/mapstruct/ap/test/collection/erronuous/ErronuousCollectionMappingTest.java
+++ b/processor/src/test/java/org/mapstruct/ap/test/collection/erroneous/ErronuousCollectionMappingTest.java
@@ -16,7 +16,7 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-package org.mapstruct.ap.test.collection.erronuous;
+package org.mapstruct.ap.test.collection.erroneous;
 
 import javax.tools.Diagnostic.Kind;
 
diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/erronuous/ErronuousMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/erroneous/ErronuousMapper.java
similarity index 95%
rename from processor/src/test/java/org/mapstruct/ap/test/collection/erronuous/ErronuousMapper.java
rename to processor/src/test/java/org/mapstruct/ap/test/collection/erroneous/ErronuousMapper.java
index c1f7b4abc..21e0b24bd 100644
--- a/processor/src/test/java/org/mapstruct/ap/test/collection/erronuous/ErronuousMapper.java
+++ b/processor/src/test/java/org/mapstruct/ap/test/collection/erroneous/ErronuousMapper.java
@@ -16,7 +16,7 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-package org.mapstruct.ap.test.collection.erronuous;
+package org.mapstruct.ap.test.collection.erroneous;
 
 import java.util.Set;