2016-10-15 3 views
0

사람들이 나에게이 시점에서 잘못된 코드가 있다고 말했습니다. 이 방법의 주요 아이디어는 사용자가 한 번 누르면 1 개의 이벤트를 만들고, 사용자가 매일 또는 매주 누를 경우 이벤트 행을 만드는 것입니다. 모든 것이 잘 작동하지만 코드가 너무 부피가 큽니다.루비 코드 개선

def create 
@event = Event.new(event_params 
@event.start_time = DateTime.parse(params[:start_time], "%Y-%m-%d %H:%i") 
@event.end_time = DateTime.parse(params[:end_time], "%Y-%m-%d %H:%i") 
@event.user_id = current_user.id 
@event.update_attributes(:repeat_id => @event.id) if @event.save 
respond_to do |format| 
    if @event.save 
    format.html { redirect_to persons_profile_path } 
    else 
    format.html { render :new } 
    format.json { render json: @event.errors, status: :unprocessable_entity } 
    end 
interval = 60 if @event.repeat =='daily' 
interval = 20 if @event.repeat =='weekly' 
#creating row of events 
if @event.repeat != 'once' 
    (1..interval).each do |i| 
    @event = Event.new(event_params) 
    @event.start_time = DateTime.parse(params[:start_time], "%Y-%m-%d %H:%i") 
    @event.end_time = DateTime.parse(params[:end_time], "%Y-%m-%d %H:%i") 
    @event.user_id = current_user.id 
    if @event.repeat =='daily' 
     @event.start_time = DateTime.parse(@event.start_time.to_s) + i.day 
     @event.end_time = DateTime.parse(@event.end_time.to_s) + i.day 
    end 
    if @event.repeat =='weekly' 
     @event.start_time = DateTime.parse(@event.start_time.to_s) + i.week 
     @event.end_time = DateTime.parse(@event.end_time.to_s) + i.week 
    end 
    @event.update_attributes(:repeat_id => k) if @event.save 
    @event.save 
    end 
end 

다른 해결책을 찾을 수 없습니다. 어떤 아이디어? 날짜에 구문 분석 방법에주의를 기울이지 말고 JS에 필요합니다.

+0

당신은 두 번째 줄에 ')'가 빠져있는 것처럼 보일 것입니다. 내 주요 질문은 코드가 작동하지 않거나 단지 비효율적으로 작성된다는 것입니다. 그것이 나중에 코드 검토로 옮겨야한다면, 코드 검토가 더 자세한 정보를 추가해야하는 경우 코드 검토로 이동해야합니다. –

+0

이 코드에는 그 의미가 혼란 스러울만큼 많은 중복이 있습니다. 가장 먼저 수행해야 할 작업 중 하나는 특정 데이터에서 'DateTime.parse'와 같은 작업을 한 번 수행 한 다음 해당 값을 다시 사용하는 것입니다. 'if'를'case'로 접어서 어떤 오프셋을 사용할 것인지를 결정한 다음, 그 오프셋을 두 값에 모두 추가 할 수 있습니다. – tadman

답변

2

저는 컨트롤러가 너무 많은 작업을 수행하고 읽을 수 없다는 것이 문제라고 생각합니다. 일반적으로 컨트롤러 모양은 다음과 같습니다.

def create 
    @client = Client.new(params[:client]) 
    if @client.save 
    redirect_to @client 
    else 
    render "new" 
    end 
end 

잠시 시간을내어 데이터를 생각해보십시오. 당신은 이벤트를 만듭니다. 작성 버튼을 매일 또는 매주 누를 경우 이벤트가 연속으로 나타나길 원합니다. 그래서 우리는 그것을 어떻게 표시 할 것인가를 생각하고 있습니다, 그것은 css/html 문제입니다. 컨트롤러와 관련된 것이 아닙니다.

사람들이 당신의 코드를 틀린 것으로 말하고있는 이유는 당신이 매우 비 전형적인 것을하고 있기 때문입니다. 학교 또는 프로젝트에이 코드를 제출하면 모든 사람들이 훈련받은 동일한 기본 스타일을 따르는 대신 다른 모든 사람들이 자신의 작업을 이해하기 어렵게 만들기 때문에 점수가 낮아집니다.