-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SWITCH #16
Add SWITCH #16
Changes from 7 commits
899f875
362ab96
2855300
51d5b9d
9a6db25
35a8e76
b1ae537
4a009cd
a230f0f
b066e5d
b9f6b7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||||
module XlsFunction | ||||||||
module Evaluators | ||||||||
module Functions | ||||||||
class Switch < ::XlsFunction::Evaluators::FunctionEvaluator | ||||||||
function_as :switch | ||||||||
|
||||||||
def eval_arglist | ||||||||
# Skip common argument evaluation and assignment for short-circuit | ||||||||
end | ||||||||
|
||||||||
def eval | ||||||||
condition = arg_list.first.evaluate(context) | ||||||||
count = 0 | ||||||||
|
||||||||
arg_list[1..-1].each_slice(2) do |expr, value| | ||||||||
count += 1 | ||||||||
break if count > 126 | ||||||||
|
||||||||
return value&.evaluate(context) if expr&.evaluate(context) == condition | ||||||||
|
||||||||
return expr&.evaluate(context) if value&.evaluate(context).nil? | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed in the commit below. |
||||||||
end | ||||||||
|
||||||||
XlsFunction::ErrorValue.na | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to add some messages with class_info. See also other ErrorValue usage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a message in the commit below. |
||||||||
end | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗒
|
||||||||
end | ||||||||
end | ||||||||
end | ||||||||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
RSpec.describe XlsFunction do | ||
subject { described_class.evaluate(input) } | ||
|
||
describe 'SWITCH' do | ||
context 'when there is a result corresponding to the matching value' do | ||
let(:input) { 'SWITCH("abc", "abc", "match", "not match")' } | ||
|
||
it { is_expected.to eq('match') } | ||
end | ||
|
||
context 'when none match' do | ||
let(:input) { 'SWITCH("abc", "zzz", "match", "not match")' } | ||
|
||
it { is_expected.to eq('not match') } | ||
end | ||
|
||
context 'when there are no matches and no default arguments are specified' do | ||
let(:input) { 'SWITCH("abc", "zzz", "match")' } | ||
|
||
it { is_expected.to eq('#N/A') } | ||
end | ||
|
||
context 'when there are more than 126 arguments' do | ||
let(:input) { 'SWITCH("abc", ' + (1..127).map { |i| "\"#{i}\", \"match\"" }.join(', ') + ', "not match")' } | ||
|
||
it { is_expected.to eq('#N/A') } | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[imo] Original one is restricted to 126 as you implemented. But I think our function does not need to be restricted as well. I don't restrict similar functions such as
ifs
andlet
. How do you think ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, I tried to reproduce it faithfully, but I also think that restrictions are unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted it in the commit below.
4a009cd