Skip to content

fiature: Ответ на часть Rack#1

Open
acidzloi wants to merge 3 commits intomainfrom
rack
Open

fiature: Ответ на часть Rack#1
acidzloi wants to merge 3 commits intomainfrom
rack

Conversation

@acidzloi
Copy link
Owner

@acidzloi acidzloi commented Sep 3, 2025

No description provided.

app.rb Outdated

false
end
end No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пора уже научиться избегать No new lina at end of file

app.rb Outdated
private

def headers
{ 'Content-Type' => 'text/plain' }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для настройки ContentType по умолчанию можно воспользоваться настройкой через Rack::ContentType

https://www.rubydoc.info/gems/rack/Rack/ContentType

и этот код будет не нужным

end

def body
return ["404\n"] unless format_exist?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

404 надо отдавать не от отсутствие какого-то параметра, а от отсутствия запрашиваемого ресурса

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

так же в rack есть инструмент для путе Rack::URLMap

format_time.rb Outdated
end

def params_valid?
return false unless (format_params - VALID_FORMAT.keys).empty?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

метод .empty? предикатный. Он и так вернёт тру или фолс.
Дополнительные проверки типа return false лишние
получается какая-то тавтология

app.rb Outdated

def status
return 404 unless format_exist?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут пустая строка лишняя.
м/у retrurn пустые строки не оставляют

app.rb Outdated

class App
def call(env)
Rack::URLMap.new(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rack::URLMap уже объявлен в config.ru

time_handler.rb Outdated
end

def headers
{ 'Content-Type' => 'text/plain' }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

как то много Content-Type по коду. Достаточно 1 раз указать в config.ru, так как другой тим не планируется передавать

time_handler.rb Outdated
@@ -0,0 +1,23 @@
class TimeHandler

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

этот классвремя не обрабатывает, а служит контроллером запроса. Название класса как App более подходило. А вот текущий app.rb очень сомнительный файл

private

def status
return 404 unless @request.path_info == '/time'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это условие ни когда не будет true. так как в конфиге уже прописан Rack::URLMap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants