activegraph
activegraph copied to clipboard
Add TypeConverter DSL
This pull introduces/changes:
- Added
Neo4j::Shared::TypeConverters.create
DSL method to create a simple converter
I noticed that most of the converters are written as classes with all their methods defined on self
and they could just be object instances with the primitive_type
and convert_type
being stored as instance variables rather than full-on methods. And since most of the type converters I write are simple conversions, I extracted most of my converters to look like this:
Neo4j::Shared::TypeConverters.create db: String, ruby: BCrypt::Password do
db(&:to_s)
ruby do |value|
if BCrypt::Password.valid_hash?(value)
BCrypt::Password.new(value)
else
BCrypt::Password.create(value)
end
end
end
In my own app, this is all within the Conversion
class, which is why it's still called that in this PR — I straight-up pasted from my own code and extracted the method to live on the TypeConverters
module. I wasn't sure if this was something people might want, so I haven't cleaned it up much, but I wanted to offer it up in case anyone found it useful.
Codecov Report
Merging #1484 into master will decrease coverage by
<.01%
. The diff coverage is96.29%
.
@@ Coverage Diff @@
## master #1484 +/- ##
==========================================
- Coverage 96.85% 96.85% -0.01%
==========================================
Files 205 205
Lines 12615 12642 +27
==========================================
+ Hits 12218 12244 +26
- Misses 397 398 +1
Impacted Files | Coverage Δ | |
---|---|---|
...ntegration/type_converters/type_converters_spec.rb | 100% <100%> (ø) |
:arrow_up: |
lib/neo4j/shared/type_converters.rb | 95.65% <93.75%> (-0.15%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 961e28c...940a9d7. Read the comment docs.
This is awesome, thank you! A couple of things:
- Could you change the documentation to reflect this DSL? I think that this would be the appropriate section
- For an example, I didn't think about the fact that
db(&:to_s)
was equivalent todb {|value| value.to_s }
, leading me to think that the whole block was uneven (I thought that you returned the DB value and defined aruby
block). It might be good in the docs to use a explicit block for whatever example it is to avoid confusion and people can optimize as they like
Could you change the documentation to reflect this DSL?
Heck yes I can.
Speaking of docs, this project has had great docs ever since I first looked at it, but they've improved a lot since 8.0 and I really appreciate your continued efforts on that front.
I didn't think about the fact that
db(&:to_s)
was equivalent todb {|value| value.to_s }
I actually meant to change that here for that exact reason and forgot before posting. The shorthand is not very expressive in this situation at all, but easy to write. My coworkers always call me out on stuff like that, too. My bad. :-)
Thanks on all counts ;) I try to operate under the assumption that is somebody has a question then the docs fell short, but there are still some holes I'd like to fill eventually ;)
And I totally do stuff like db(&:to_s)
myself and am fine with it, I just think the other way would be more accessible (though I don't think I need to explain this, given your comments ;) )