Skip to content
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

Allow hash surrealization #106

Merged
merged 4 commits into from
Jun 12, 2018

Conversation

stefkin
Copy link
Contributor

@stefkin stefkin commented May 29, 2018

Related issue: #96

Since version in master traverses hashes just fine when they're located deeper in object tree the issue with Hashes appears only when root object in a Hash. Instead of creating a HashBuilder for pure hashes, I tried to fix the existing process to allow surrealization of mixed data structures with Hash in the root.

Hash was misinterpreted as a collection by Helper#collection? and ValueAssigner#raw_value doesn't peek into instance's object while trying to extract value with Hash#[].
I tried to fix that in an ad-hoc manner, I hope it's not too bad.
I think it would be better to rework value extraction in ValueAssigner, but I'm afraid value extraction logic can become too messy.

Anyway, I'm ready to discuss a better approach to resolve the issue.


context 'root hash with object inside' do
specify do
expect(HashRoot.new({nested: OpenStruct.new(real: 1, imaginary: -1)}).build_schema).to eq({nested: { real: 1, imaginary: -1}})

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.
Metrics/LineLength: Line is too long. [134/110]


context 'deep hash arg' do
specify do
expect(DeepHash.new({list: [1, 2], nested: { right: 4, left: 'three'}}).build_schema).to eq({list: [1, 2], nested: { right: 4, left: 'three'}})

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.
Metrics/LineLength: Line is too long. [151/110]

RSpec.describe Surrealist do
describe '#build_schema' do
context 'with hash arg' do
specify do
expect(ComplexNumber.new({real: 1, imaginary: 2}).build_schema).to eq(real: 1, imaginary: 2)

Choose a reason for hiding this comment

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

Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.

@nesaulov
Copy link
Owner

nesaulov commented Jun 2, 2018

@stefkin thank you for the PR!
This implementation seems fine to me, but could you please add specs for the additional features (like arguments to #surrealize) so we can be sure that everything works as it should. And please also fix rubocop issues

it { is_expected.to eq expectation }
end

describe "#surrealize with camelize" do

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

it { is_expected.to eq expectation }
end

describe "#surrealize with include root" do

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

let(:flour) { Struct.new(:amount).new(40) }
let(:instance) { { color: 'yellow', amount: 60 } }

describe "#surrealize" do

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

@stefkin
Copy link
Contributor Author

stefkin commented Jun 6, 2018

@nesaulov I've added a couple of tests using #surrealize with different args and fixed cops.

Copy link
Owner

@nesaulov nesaulov left a comment

Choose a reason for hiding this comment

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

@stefkin looks good, thank you!

@nesaulov nesaulov merged commit 88d4e48 into nesaulov:master Jun 12, 2018
@nesaulov nesaulov mentioned this pull request Jun 12, 2018
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.

3 participants