Это спагетти-код?


Недавно я написал свой первый webapp и вот код. Я хочу сделать его лучше, но я точно не знаю, как. Я считаю, что в первую очередь я должна начать с его структуры. Это спагетти-код? Если да, то как мне это изменить? Вы можете посмотреть его на http://shiftfrog.com

Я создал класс под названием 'делатель' что в принципе движет программу. Что не так? Должен ли мой класс можно назвать календарь, а затем манипулировать в моем посте призыв /календарь?

class Doer

  def makeDate(date)
    return Date.strptime(date, "%m/%d/%Y")
  end

  def buildArray(dateObj, on, off)
    array = frontpadding(dateObj, on, off)
    month = dateObj.month
    newMonth = month
    day = dateObj.mday
    date = dateObj
    while month == newMonth
      temp_array = [day,'day']
      array.push(temp_array)
      day = day + 1
      date = date + 1
      newMonth = date.month
    end
    endpadding(date, array)
    array
  end

  def endpadding(dateObj, array)
    dofw = dateObj.wday
    filler = (dofw - 6).abs + 1
    if filler == 7
      #do nothing
    else
      until filler == 0
        temp_array = ['','day']
        array.push(temp_array)
        filler = filler - 1
      end
    end
  end

  def frontpadding(dateObj, on, off)
    array = Array.new
    day = dateObj.mday 
    firstOfMonth = Date.new(dateObj.year, dateObj.month)
    filler = firstOfMonth.wday 
    on = on.to_i
    off = off.to_i
    mod = on + off
    if day != 1
      @days = day
      @location = @days + filler
      until day <= 1
        off.times do
          if day > 1
            temp_array = ['','day']
            array.unshift(temp_array)
            day = day - 1
          end
        end
        on.times do
          if day > 1
            temp_array = ['','dayOn']
            array.unshift(temp_array)
            day = day - 1
          end
        end
      end
    end 
    until filler == 0
        temp_array = ['','day']
        array.unshift(temp_array)
        filler = filler - 1
    end    
    array
  end

  def backFill(cal, on, off)
    @location = @location.to_i
    if @location > 0
      @location = @location - 1
      until @days == 0
        cal[0][1][@location][0] = @days
        @days = @days - 1
        @location = @location - 1
      end
    end
    cal
  end

  def makeCal(date, on, off)
    dateObj = makeDate(date)

    @cal = Array.new
    months = 0
    while months < 13
    #   #pass dateobj to build array
      array = buildArray(dateObj, on, off)
    #   #save array to hash with month key
      monthName = Date::MONTHNAMES[dateObj.mon]
      monthYear = "#{monthName} " + dateObj.year.to_s
      holder = Array.new
      holder.push monthYear
      holder.push array
      @cal.push holder
    #   #create new date object using month and set it to the first
      dateObj = Date.new(dateObj.year, dateObj.month)
      dateObj >>= 1
      months = months + 1
    end
    @cal = createCal(@cal, on, off)
    @cal = backFill(@cal, on, off)
  end

  def createCal(cal, on, off)
    on = on.to_i
    off = off.to_i
    @mod = on + off
    daycount = 0
    cal.each do |month|
      month[1].each do |day|
        if day[0] == ''
          #do nothing
        else
          if daycount % @mod < on
            day[1] = 'dayOn'          
          end
          daycount = daycount + 1 
        end
      end
    end       
    cal
  end
end

get '/' do
  haml :ask
end

post '/calendar' do
  @on = params["on"]
  @off = params["off"]
  @date = params["date"]


  a = Doer.new
  @cal = a.makeCal(@date, @on, @off)
  haml :feeling
end


623
4
задан 11 сентября 2011 в 02:09 Источник Поделиться
Комментарии
1 ответ

Делатель не особо выразительное имя. Конечно, класс делает что-то ... но какой класс не? Ваш порыв назовите класс календарь определенно в цель. Есть несколько других имен вещей, которые не по Руби стиль руководства; следует избегать CamelCasing ваш метод имена-например, makeCal следует make_cal а еще лучше как-то так сделать! или создать! (если класс сам по имени календаря, заявив @календаря.make_calendar повторяет себя.)

В целом, по отношению к "спагетти" кода, проблема, как правило, что вы на самом деле не разделяя озабоченности. Проблема в том, что объекты пытаются решить инкапсуляция ответственности. Он уничтожает любые преимущества вы стоите, чтобы получить весь функционал в одном месте. Объектно-ориентированное программирование предполагает модульность и инкапсуляция -- наименовании класса делатель может указывать на некоторые недостатки в плане действительно нашарили парадигмы. Рассмотрели некоторые актуальные исходный код веб-приложения и увидеть, как опасения разделяются и обрабатываются на соответствующем уровне абстракции.

По последнему пункту, одно хорошее правило большого пальца заключается в том, что код, который "рядом" должен работать примерно на одном уровне абстракции. Попытаться определить различные проблемы и распределить их ... сделать их 'слабосвязанных' друг с другом. Вы могли бы сделать класс, который обрабатывает бизнес-логику обивка и упаковка в строку вещи и отделять "чистый" календарь логику в другой класс в целом; ваша заявка будет дальше остается только "клей" две функции вместе.

6
ответ дан 24 сентября 2011 в 01:09 Источник Поделиться