Groovy/Grails 코드 정리 제안, 제발!

Question

Groovy를 처음 접했을 때 & Grails, 나는 이런 것들이 추악 할 필요가 없다는 느낌이 들었습니다 ... 어떻게이 코드를 더 멋지게 만들 수 있습니까?

이것은 흥미로운 비트를 뺀 Grails 컨트롤러 클래스입니다. 내가 나중에 :-)

changeWheel 아약스 액션이 처리 할 수 ​​- 너무 내 Car는 하나의 Wheel을 가지고 끊었 얻을 수 없습니다보십시오.

class MyController { 
    ... 
    def changeWheel = { 
     if(params['wheelId']) { 
      def newWheel = Wheel.findById(params['wheelId']) 
      if(newWheel) { 
       def car = Car.findById(params['carId']) 
       car?.setWheel(newWheel) 
       if(car?.save()) render 'OK' 
      } 
     } 
    } 
} 

Answer 1

1) 밖으로 자신의 인증 된 정의에

2) 다수의 중첩 된 IFS는 결코 최적

params['carId'] 

params['wheelId'] 

및

를 당깁니다. wheelId 및 carId가 설정되지 않은 경우 validateParams 메소드를 사용하고 응답을 렌더링하여 가장 바깥 쪽을 제거 할 수 있습니다. 아니면 그냥 코드 자체를 만들기 위해, 마지막으로) 그냥이 더 중첩 된 구조를 제거한다

def newWheel = Wheel.findById... 
def car = Car.findById... 
if (car != null && newWheel != null) { 
    car.setWheel(newWheel) 
    car.save() 
    render 'OK' 
} else { 
    // either wheel or car is null 

} 

...

4 할 수

if (carId == null || wheelId == null) { 
    // params invalid 
} 

3) 가정하면 모든 것이 괜찮아 할 조건부 테스트를 적절한 이름의 변수에 할당하는 등의 작업을 수행 할 수 있습니다. 그래서 뭔가 같은

def carAndWheelOk = car != null && newWheel != null 
if (carAndWheelOk) { 
    // do the save 
} else { 
    // car or wheel not ok 
} 

이것은 두 개의 테스트를 위해 과잉 일 수 있습니다. 네 바퀴 모두를 다루고 있다면 이런 종류의 것들은 가독성과 유지 보수성을 높여줍니다.

이 조언은 모든 언어에서 작동합니다. 그루비의 통사론으로 당신이 너무 많이 할 수 있다고는 생각하지 않지만 어쩌면 그루비 한 전문가가 더 나은 조언을 해줄 수 있습니다.

Answer 2

실제로는 Command Objects을 사용하고 있습니다.

이 시도 :보기 사용 'car.id'대신 'carId'의 'wheel.id'와 'wheelId'에서 다음

class MyController { 
    def index = { 
    } 
    def changeWheel = { CarWheelCommand cmd -> 
     if(cmd.wheel && cmd.car) { 
      Car car = cmd.car 
      car.wheel = cmd.wheel 
      render car.save() ? 'OK' : 'ERROR' 
     } else { 
      render "Please enter a valid Car and wheel id to change" 
     } 
    } 
} 
class CarWheelCommand { 
    Car car 
    Wheel wheel 
} 

및

당신이 좋아 할 수있는 몇 가지가 있습니다

Answer 3

일부 코드를 서비스 또는 명령 객체로 이동합니다. '[대신 PARAMS의 (params.wheelId를 PARAMS 값을 참조하는

1. 사용 점 표기법 대신 배열 색인 :하지만 너무 많은 구조를 변경하지 않고, I (주관적)는 다음과 쉽게 코드를 읽을 수 있도록 것이라고 생각 wheelId '])

2. 중첩을 줄이려면 if를 뒤집을 것이므로 예외가 무엇인지 명확하게 알 수 있다고 생각합니다.

   예를 들어

: 당신이 구조를 변경하고 코드를 더 줄을 추가 괜찮다면 지금

if(!params.wheelId) { 
    sendError(400, "wheelId is required") 
    return 
} 
.... 
.... 
if(!newWheel) { 
    sendError(404, "wheel ${params.wheelId} was not found.") 
    return 
} 

은 ...

는 바퀴를 변경하는 행위는 공통 될 수있다 하나 이상의 컨트롤러 동작 이상에서 발생합니다. 이 경우에는 GORM/데이터베이스 로직을 Service 클래스에 두는 것이 좋습니다. 그런 다음 컨트롤러가 정확한 매개 변수 입력을 확인하고 실제 타이어를 변경하기 위해 서비스에 전달합니다. 서비스 방법은 트랜잭션 방식 일 수 있습니다. 새 타이어를 장착하기 전에 오래된 타이어를 분리해야 할 수도 있습니다.

서비스에서는 바퀴가 발견되지 않거나 차를 찾지 못하거나 타이어를 교체하는 중에 오류가 발생하는 경우와 같은 예외적 인 경우에는 예외를 던질 것입니다. 그런 다음 컨트롤러가 해당 컨트롤러를 포착하여 적절한 HTTP 상태 코드로 응답 할 수 있습니다.