Comments (10)
Dan has fixed this :)
from virtus.
This was fixed in 86c0dc2
@solnic wdyt about a 0.0.9 release with this fix?
from virtus.
+1 for a 0.0.9 release (because I'm selfish and need it :P)
from virtus.
sure but first we need to clean up that type lookup a bit cause our metric tools aren't happy:
lib/virtus/support/type_lookup.rb -- 3 warnings:
Virtus::TypeLookup#determine_type_from_primitive calls descendant.primitive twice (Duplication)
Virtus::TypeLookup#determine_type_from_primitive refers to descendant more than self (LowCohesion)
Virtus::TypeLookup#determine_type_from_primitive refers to type more than self (LowCohesion)
and
lib/virtus/support/type_lookup.rb:69 - Block cyclomatic complexity is 4. It should be 2 or less.
from virtus.
Virtus::TypeLookup#determine_type_from_primitive calls descendant.primitive twice (Duplication)
This looks fine to me. It's not duplication of logic, it's just avoiding temporary variables (a code smell itself):
def determine_type_from_primitive(primitive)
type = nil
descendants.reverse_each do |descendant|
next unless primitive <= descendant.primitive
type = descendant if type.nil? || type.primitive > descendant.primitive
end
type
end
Virtus::TypeLookup#determine_type_from_primitive refers to descendant more than self (LowCohesion)
Virtus::TypeLookup#determine_type_from_primitive refers to type more than self (LowCohesion)
Again, the only way I see to fix those is to copy descendant.primitive
and type.primitive
into a temporary variable, which, IMHO is not something that should be encouraged. Unnecessary temporary variables make refactoring harder further down the line.
I'm not seeing the same warning about the cyclomatic complexity. I assume somebody already changed this?
from virtus.
@d11wtq I tried to quickly simplify this method and failed. this isn't blocking the release of course, so if we can't come up with a better implementation now I will release 0.0.9 so the bug that affects you is gone and do some small refactoring for 0.1.0. I'll push 0.0.9 later today.
from virtus.
You could define methods type_primitive
and descendant_primitive
, but that doesn't really simplify anything and only serves to trick reek ;)
from virtus.
PS: Thanks for considering me in your release schedule! I can't give enough praise to Virtus. I love it. Makes coding against a remote API way more fun :P
from virtus.
One refactoring for the future would be to move the responsibility of making the "best match" into the descendants object.
Currently the descendants list is an Array, because I didn't need anything more complex, but given that other parts of the project are using descendants, perhaps it's time to make a minimally complete object that can be appended to, and knows how to match a class within itself. I'm not too comfortable exposing something important as part of an API that's made up solely of primitive objects.
I might also consider making a method either on the Descendants or Attribute that knows how to do the "best match"; it will either return the current Attribute object, or the one provided, whichever is more specific.
from virtus.
I should add too that I don't think such a refactoring should be a release blocker.
from virtus.
Related Issues (20)
- Publish 1.0.6
- Avoid override of previously assigned attributes
- Put link to dry-rb in README.md to save time for new comers
- possible to dynamically add attributes? HOT 1
- What replaces `ValueObject::InstanceMethods::with`?
- Class not getting initialized HOT 1
- Bug: nil default value of Array attribute ignored HOT 1
- Coerce proc and strict mode don't match
- Default values aren't assigned when initialize method is present. HOT 1
- Array of Array of FooClass
- Strange Boolean behaviour HOT 5
- Validation from class attributes
- Show Embedded value on edit form
- Calling methods inside of a Virtus::Attribute
- Mark as unmaintaned HOT 2
- Hash.try_convert(attributes) errors on params in Rails 5
- FixedWidth Coercion HOT 2
- Date formats HOT 1
- How can I use array of tags(Strings) in a key of a hash? HOT 1
- Strict mode only works for basic values HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from virtus.