From 24530121a81bfd867ef5bb23ec28d988e6a40fc4 Mon Sep 17 00:00:00 2001 From: netznarkose Date: Wed, 18 Jan 2017 18:22:58 +0100 Subject: [PATCH 1/3] add new validation for options 1. colors must be greater then number of data and: 2. labels must equal number of data --- lib/charts/bar_chart/bar_chart.rb | 2 +- lib/charts/chart.rb | 47 ++++++++++++------- spec/charts/bar_chart/bar_chart_spec.rb | 6 +-- spec/charts/count_chart/count_chart_spec.rb | 50 ++++++++++----------- 4 files changed, 59 insertions(+), 46 deletions(-) diff --git a/lib/charts/bar_chart/bar_chart.rb b/lib/charts/bar_chart/bar_chart.rb index 941f8d8..fb34c39 100644 --- a/lib/charts/bar_chart/bar_chart.rb +++ b/lib/charts/bar_chart/bar_chart.rb @@ -53,7 +53,7 @@ def pre_draw def draw_group_labels return if options[:group_labels].nil? || group_labels.empty? - raise ArgumentError if group_labels.count != group_count + raise ArgumentError, 'count of group-Labels and bars does not match' if group_labels.count != group_count group_label_style = { text_anchor: 'middle', writing_mode: (vertical? ? 'lr' : 'tb'), diff --git a/lib/charts/chart.rb b/lib/charts/chart.rb index 699e5fc..216a1aa 100644 --- a/lib/charts/chart.rb +++ b/lib/charts/chart.rb @@ -1,8 +1,8 @@ class Charts::Chart attr_reader :data, - :options, - :prepared_data, - :renderer + :options, + :prepared_data, + :renderer def initialize(data, opts = {}) validate_arguments(data, opts) @@ -14,23 +14,37 @@ def initialize(data, opts = {}) end def validate_arguments(data, options) - raise ArgumentError.new('Data missing') if data.empty? - raise ArgumentError.new('Data not an array') unless data.is_a? Array - raise ArgumentError.new('Options missing') unless options.is_a? Hash - if options[:outer_margin] and !options[:outer_margin].is_a?(Numeric) - raise ArgumentError.new('outer_margin not a number') + raise ArgumentError, 'Data missing' if data.empty? + raise ArgumentError, 'Data not an array' unless data.is_a? Array + raise ArgumentError, 'Options missing' unless options.is_a? Hash + if options[:outer_margin] && !options[:outer_margin].is_a?(Numeric) + raise ArgumentError, 'outer_margin not a number' end - validate_array_and_count(data, options, :colors) - validate_array_and_count(data, options, :labels) + is_an_array?(options, [:labels, :colors]) + validate_colors(data, options, :colors) + validate_labels(data, options, :labels) end - def validate_array_and_count(data, options, key) + def is_an_array?(options, keys) + keys.map do |key| + if options[key] && !options[key].is_a?(Array) + raise ArgumentError, "#{key} not an array" + end + end + end + + def validate_colors(data, options, key) if options[key] - unless options[key].is_a? Array - raise ArgumentError.new("#{ key } not an array") + if options[key].any? && data.count > options[key].count + raise ArgumentError, "number of #{key} is too small" end - if options[key].any? and data.count > options[key].count - raise ArgumentError.new("not enough #{ key }") + end + end + + def validate_labels(data, options, key) + if options[key] + if options[key].any? && (data.count != options[key].count) + raise ArgumentError, "number of #{key} does not match array" end end end @@ -91,8 +105,7 @@ def draw_title renderer.text options[:title], x, y, text_anchor: 'middle', class: 'title' end - def initialize_instance_variables - end + def initialize_instance_variables; end def create_options_methods options.each do |key, value| diff --git a/spec/charts/bar_chart/bar_chart_spec.rb b/spec/charts/bar_chart/bar_chart_spec.rb index 08acefd..b839ed5 100644 --- a/spec/charts/bar_chart/bar_chart_spec.rb +++ b/spec/charts/bar_chart/bar_chart_spec.rb @@ -243,7 +243,7 @@ describe 'too few group_labels' do let(:group_labels) { ['one', 'two'] } it 'raises an error' do - expect{ chart.render }.to raise_error(ArgumentError) + expect{ chart.render }.to raise_error(ArgumentError, 'count of group-Labels and bars does not match') end end end @@ -255,10 +255,10 @@ labels_texts = svg.css('text.label').map{ |t| t.text.tr("\n", '') } expect(labels_texts).to eq(labels) end - describe 'too few labels' do + describe 'count of label and data does not match' do let(:labels) { ['Alpha', 'Beta'] } it 'raises an error' do - expect{ chart.render }.to raise_error(ArgumentError) + expect{ chart.render }.to raise_error(ArgumentError, 'number of labels does not match array') end end end diff --git a/spec/charts/count_chart/count_chart_spec.rb b/spec/charts/count_chart/count_chart_spec.rb index ab14252..8693dac 100644 --- a/spec/charts/count_chart/count_chart_spec.rb +++ b/spec/charts/count_chart/count_chart_spec.rb @@ -42,16 +42,16 @@ let(:chart) { Charts::CountChart.new [1] } it 'has a default item-colors' do expect(chart.colors).to eq([ - '#e41a1d', - '#377eb9', - '#4daf4b', - '#984ea4', - '#ff7f01', - '#ffff34', - '#a65629', - '#f781c0', - '#888888' - ]) + '#e41a1d', + '#377eb9', + '#4daf4b', + '#984ea4', + '#ff7f01', + '#ffff34', + '#a65629', + '#f781c0', + '#888888' + ]) end it 'has a default background-colors' do expect(chart.background_color).to eq('white') @@ -176,23 +176,23 @@ it 'creates the prepared_data for simple keys' do chart = Charts::CountChart.new([3, 2], colors: ['x', 'o'], columns: 2) expect(chart.prepared_data).to eq([ - ['x', 'x'], - ['x', 'o'], - ['o'] - ]) + ['x', 'x'], + ['x', 'o'], + ['o'] + ]) end it 'creates the prepared_data for complex keys' do chart = Charts::CountChart.new([2, 2], colors: ['#FF0000', '#00FF00'], columns: 2) expect(chart.prepared_data).to eq([ - ['#FF0000', '#FF0000'], - ['#00FF00', '#00FF00'] - ]) + ['#FF0000', '#FF0000'], + ['#00FF00', '#00FF00'] + ]) end it 'default colors get assigned when no colors are specified' do chart = Charts::CountChart.new([1, 1, 1]) expect(chart.prepared_data).to eq([ - ['#e41a1d', '#377eb9', '#4daf4b'] - ]) + ['#e41a1d', '#377eb9', '#4daf4b'] + ]) end end @@ -251,23 +251,23 @@ describe '#height and #width with presence of labels' do context 'three items with three labels' do - let(:data) { [3] } + let(:data) { [3, 3, 3] } let(:labels) { ['Cars', 'Buses', 'Bikes'] } let(:item_height) { 20 } - include_examples 'has a width and height of', 40, 120 + include_examples 'has a width and height of', 40, 180 end context 'five items with five labels' do - let(:data) { [5] } + let(:data) { [5, 5, 5, 5, 5] } let(:labels) { ['Cars', 'Buses', 'Bikes', 'Planes', 'Ferries'] } let(:item_height) { 20 } - include_examples 'has a width and height of', 40, 180 + include_examples 'has a width and height of', 40, 380 end context 'five items in three columns with five labels' do - let(:data) { [5] } + let(:data) { [5, 5, 5, 5, 5] } let(:columns) { 3 } let(:labels) { ['Cars', 'Buses', 'Bikes', 'Planes', 'Ferries'] } let(:item_height) { 20 } - include_examples 'has a width and height of', 60, 160 + include_examples 'has a width and height of', 60, 300 end end From cec4807188dc76dafb995f7a93923b54cff78b5c Mon Sep 17 00:00:00 2001 From: netznarkose Date: Wed, 18 Jan 2017 19:39:39 +0100 Subject: [PATCH 2/3] rollback to old ArgumentErrorMessage syntax --- lib/charts/chart.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/charts/chart.rb b/lib/charts/chart.rb index 216a1aa..dc0781b 100644 --- a/lib/charts/chart.rb +++ b/lib/charts/chart.rb @@ -14,9 +14,9 @@ def initialize(data, opts = {}) end def validate_arguments(data, options) - raise ArgumentError, 'Data missing' if data.empty? - raise ArgumentError, 'Data not an array' unless data.is_a? Array - raise ArgumentError, 'Options missing' unless options.is_a? Hash + raise ArgumentError.new('Data missing') if data.empty? + raise ArgumentError.new('Data not an array') unless data.is_a? Array + raise ArgumentError.new('Options missing') unless options.is_a? Hash if options[:outer_margin] && !options[:outer_margin].is_a?(Numeric) raise ArgumentError, 'outer_margin not a number' end From 9491a6418b3f99ccda8ce1941689730fc5e7a91e Mon Sep 17 00:00:00 2001 From: netznarkose Date: Wed, 18 Jan 2017 22:04:44 +0100 Subject: [PATCH 3/3] replace empty single-line-method with a good old two-liner --- lib/charts/chart.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/charts/chart.rb b/lib/charts/chart.rb index dc0781b..d3b5866 100644 --- a/lib/charts/chart.rb +++ b/lib/charts/chart.rb @@ -105,7 +105,8 @@ def draw_title renderer.text options[:title], x, y, text_anchor: 'middle', class: 'title' end - def initialize_instance_variables; end + def initialize_instance_variables + end def create_options_methods options.each do |key, value|