Диан обнови решението на 20.12.2013 21:19 (преди почти 11 години)
+module Graphics
+ module Renderers
+ class Html
+ end
+
+ class Ascii
+ end
+
+ HTML_PREFIX = "<!DOCTYPE html>
+ <html>
+ <head>
+ <title>Rendered Canvas</title>
+ <style type=\"text/css\">
+ .canvas {
+ font-size: 1px;
+ line-height: 1px;
+ }
+ .canvas * {
+ display: inline-block;
+ width: 10px;
+ height: 10px;
+ border-radius: 5px;
+ }
+ .canvas i {
+ background-color: #eee;
+ }
+ .canvas b {
+ background-color: #333;
+ }
+ </style>
+ </head>
+ <body>
+ <div class=\"canvas\">\n"
+ HTML_SUFFIX = " </div>
+ </body>
+ </html>\n"
+ end
+ class Point
+ attr_reader :x, :y, :pixel_container
+ def initialize(x, y)
+ @x = x
+ @y = y
+ @pixel_container = Array.new
+ @pixel_container << self
+ end
+
+ def ==(point)
+ @x == point.x and @y == point.y
+ end
+
+ alias_method :eql?, :==
+
+ def hash
+ [@x, @y].hash
+ end
+
+ def get_min_x(point)
+ if @x <= point.x then self else point end
+ end
+
+ def get_min_y(point)
+ if @y <= point.y then self else point end
+ end
+
+ def get_max_x(point)
+ if @x > point.x then self else point end
+ end
+
+ def get_max_y(point)
+ if @y > point.y then self else point end
+ end
+ end
+
+ class Line
+ attr_reader :from, :to, :pixel_container
+ def initialize(point_a, point_b)
+ initialize_line_ends(point_a, point_b)
+ @pixel_container = Array.new
+ if point_a.x != point_b.x then generate_standard else generate_vertical end
+ end
+
+ def ==(line)
+ @from == line.from and @to == line.to
+ end
+
+ alias_method :eql?, :==
+
+ def hash
+ [@from, @to].hash
+ end
+
+ private
+ def initialize_line_ends(point_a, point_b)
+ if point_a.x != point_b.x
+ @from = point_a.get_min_x point_b
+ @to = point_a.get_max_x point_b
+ else
+ @from = point_a.get_min_y point_b
+ @to = point_a.get_max_y point_b
+ end
+ end
+
+ def generate_standard
+ delta_error = (Float(@to.y - @from.y) / Float(@to.x - @from.x)).abs
+ error, y = 0, @from.y
+ (@from.x..@to.x).each do |x|
+ @pixel_container << Point.new(x, y)
+ error, y = get_new_error_and_ordinate error, delta_error, y
+ end
+ @pixel_container += Line.new(Point.new(@to.x, y), @to).pixel_container
+ end
+
+ def get_new_error_and_ordinate(error, delta_error, y)
+ error += delta_error
+ if error >= 0.5
+ if @from.y < @to.y then y += 1 else y -= 1 end
+ error -= 1.0
+ end
+ return error, y
+ end
+
+ def generate_vertical
+ (@from.y..@to.y).each do |y|
+ @pixel_container << Point.new(@from.x, y)
+ end
+ end
+ end
+
+ class Rectangle
+ attr_reader :left, :right, :pixel_container
+ attr_reader :top_left, :top_right, :bottom_left, :bottom_right
+ def initialize(point_a, point_b)
+ @pixel_container = Array.new
+ initialize_ends(point_a, point_b)
+ create_ends
+ generate_pixels
+ end
+
+ def ==(rectangle)
+ @top_left == rectangle.top_left and
+ @top_right == rectangle.top_right and
+ @bottom_left == rectangle.bottom_left and
+ @bottom_right == rectangle.bottom_right
+ end
+
+ alias_method :eql?, :==
+
+ def hash
+ [@top_left, @top_right, @bottom_left, @bottom_right].hash
+ end
+
+ private
+ def initialize_ends(point_a, point_b)
+ if point_a.x != point_b.x
+ @left = point_a.get_min_x point_b
+ @right = point_a.get_max_x point_b
+ else
+ @left = point_a.get_min_y point_b
+ @right = point_a.get_max_y point_b
+ end
+ end
+
+ def create_ends
+ left_x = Point.new(@left.x, @right.y)
+ right_x = Point.new(@right.x, @left.y)
+
+ @top_left = @left.get_min_y left_x
+ @bottom_left = @left.get_max_y left_x
+
+ @top_right = @right.get_min_y right_x
+ @bottom_right = @right.get_max_y right_x
+ end
+
+ def generate_pixels
+ @pixel_container += Line.new(@top_left, @bottom_left).pixel_container
+ @pixel_container += Line.new(@bottom_left, @bottom_right).pixel_container
+ @pixel_container += Line.new(@bottom_right, @top_right).pixel_container
+ @pixel_container += Line.new(@top_right, @top_left).pixel_container
+ end
+ end
+
+ class ArrayTwoDimensional
+ def initialize(first_dimension, second_dimension)
+ @data = Array.new(first_dimension) { Array.new(second_dimension) }
+ end
+
+ def [](x, y)
+ @data[x][y]
+ end
+
+ def []=(x, y, value)
+ @data[x][y] = value
+ end
+ end
+
+ class Canvas
+ attr_reader :width, :height
+ def initialize(width, height)
+ @width = width
+ @height = height
+ @pixel_matrix = ArrayTwoDimensional.new(width, height)
+ end
+
+ def set_pixel(x, y)
+ @pixel_matrix[x, y] = 1
+ end
+
+ def pixel_at?(x, y)
+ nil != @pixel_matrix[x, y]
+ end
+
+ def draw(object)
+ object.pixel_container.each { |pixel| set_pixel(pixel.x, pixel.y) }
+ end
+
+ def render_as(renderer_type)
+ if renderer_type == Renderers::Ascii
+ render_as_ascii
+ else
+ render_as_html
+ end
+ end
+
+ private
+ def render_as_ascii
+ ascii_string = ""
+ (0...@height).each do |y|
+ (0...@width).each do |x|
+ ascii_string += ascii_pixel_check(x, y)
+ end
+ ascii_string += "\n" if y < @height - 1
+ end
+ ascii_string
+ end
+
+ def ascii_pixel_check(x, y)
+ if pixel_at?(x, y)
+ "@"
+ else
+ "-"
+ end
+ end
+
+ def render_as_html
+ html_string = Renderers::HTML_PREFIX
+ (0...@height).each do |y|
+ (0...@width).each do |x|
+ html_string += html_pixel_check(x, y)
+ end
+ html_string += "<br>\n" if y < @height - 1
+ end
+ html_string += Renderers::HTML_SUFFIX
+ end
+
+ def html_pixel_check(x, y)
+ if pixel_at?(x, y)
+ "<b></b>"
+ else
+ "<i></i>"
+ end
+ end
+ end
+end
Бележки:
- В Ruby няма практика да има думата
get_
в името на методите; тази дума просто генерира шум и не носи информация. - Лош стил е да се създава списък с
Array.new
, когато може да се напише просто[]
. Пак там, спокойно можеш да напишеш@pixel_container = [self]
. - Трябва да има нов ред след
attr_reader
дефинициите, по конвенция. - Добре си се справил с реализацията на
hash
методите. - Използваш синоними на методи по подходящ начин в случая.
- HTML кодът е добре сложен в константи, но тези константи е най-подходящо да се намират в класа
Html
. - "Prefix" и "suffix" са по-неподходящи имена, отколкото "header" и "footer" за HTML кода в тази задача.
-
pixel_container
не ми се струва като добро име; потърси по-подходящо. - Не ми се струва добра идея да имаш методи
Point#get_min_x
,get_min_y
и т.н. По-скоро ползвай елементарната им логика там, където ти трябва, директно, или намери друг начин да си свършиш работата (ternary оператор,[x, y].min/max
, ...) - Според нашите конвенции, след
private
се оставя един празен ред и методите не се вкарват едно допълнително ниво на идентация навътре. -
generate_pixels
може да се напише така:[ @top_left, @top_right, @top_right, @bottom_right, @bottom_right, @bottom_left, @bottom_left, @top_left, ].each_slice(2).map { |a, b| Line.new(a, b).pixel_container }.flatten
И след това да се кръсти направо
pixel_container
(или каквото ново име си измислил за това). - Рядко се ползва
if then else end
конструкцията. Предпочитай нормалниif/else/end
на няколко реда. В краен случай, за много прости изрази, може и ternary оператораcondition ? foo : bar
. - Вместо думата "generate", ми се струва, че ще е по-удачно да ползваш нещо друго. Например "rasterize". Имаш възможност да подобриш имена и на други места, въпреки, че виждам, че се стараеш да именуваш добре и някъде се е получило. Просто винаги може повече.
- Мисля, че "delta_error" е по-лошо име от "error_delta". Не прави int -> float така. Ползвай
to_f
метода на числата. - Нека да напомня, че празните редове не се броят от skeptic и е добра практика да се слагат на места в методи, за да разделят блокове от код.
- Има ли смисъл да проверяваш за съвпадение на всички ъгли в сравнението на правоъгълници?
- Помисли дали няма друг по-оптимален начин в Ruby за реализация вътрешното представяне на пано от масив от масиви. Ruby не е C и има по-удобни структури от данни :) Тогава ще отпадне и нуждата от този странен
ArrayTwoDimentional
клас. - Също така,
pixel_at?
може да връща нещо, което се оценява като истина или лъжа; не е задължително да еtrue
/false
. От друга страна, смятам, че е по-добре да ползвашtrue
, за да кажеш "има нещо", отколкото1
. - Пробвай да направиш методите за рендериране с
map
иjoin
. - Името
html_pixel_check
не е добро. Не изразява точно какво прави методът. Потърси друго. - Помисли кой трбява да носи отговорността за "рендериране" на пано. Самото пано или рендерера? Помисли кой обект каква отговорност носи. Една основна идея на ОО-програмирането е да разпределиш различните отговорности между различни обекти. Стреми се всеки обект да отговаря за едно конкретно нещо и само за него. Напълно нормално е да имаш голям брой малки обекти в системата, с тясно специализирани отговорности. Казвам това в контекста на празните класове
Html
иAscii
.
Общото ми впечатление е, че си на правилна посока, но има нужда да подаботиш над стила си, следването на конвенциите, именуването на променливи и стремежа към по-идиоматичен Ruby код.