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

Adds DescendantOf #227

Merged
merged 1 commit into from
Feb 24, 2016
Merged

Conversation

naps62
Copy link
Contributor

@naps62 naps62 commented Feb 23, 2016

This adds the ability to make assertions on the class hierarchy of a class being passed as an argument. It should be useful when using things like the Adapter Pattern or Dependency Injection.

Consider the following example:

class SlackClientAdapter < BaseClient
end

class HipchatClientAdapter < BaseClient
end

Contract C::DescendantOf[BaseClient] => BaseClient
def start_client(client_class)
  @client = client_class.new
end

start_client(SlackClientAdapter)
start_client(HipchatClientAdapter)

This is a demonstration of what I'm actually doing on a real app (although the example itself is totally made up).

I want to be able to assert that the argument is not just a class, but a descendant of BaseClient (this base client serves as an abstract class this example. God, I feel like a Java developer all over again...)

I'm not sure if this is something that's already been proposed and discarded, so I didn't take a lot of care in the organization of the code I added.
Should this be accepted, I'll gladly make any required changes before merging.

@egonSchiele
Copy link
Owner

Hi @naps62 ,
Actually, that's how things currently work...you don't need DescendantOf for this functionality. For example:

require "contracts"
include Contracts

class Animal; end
class Dog < Animal; end
class Cat < Animal; end

Contract Animal => Any
def foo x
  p x
end

foo Dog.new # works, Dog is an Animal (checked with is_a?)
foo "hello" # fails, String is not an Animal

The check happens here.

If you want to do the opposite (i.e. accept Animal only, not Dog or Cat), use Exactly:

Contract Exactly[Animal] => Any
def foo x
  p x
end

@waterlink
Copy link
Collaborator

Not exactly, here the argument is not an instance of class, but class itself.

Dog.new as opposed to just Dog.

Am 23.02.2016 um 11:17 PM schrieb Aditya Bhargava notifications@github.com:

Hi @naps62 ,
Actually, that's how things currently work...you don't need DescendantOf for this functionality. For example:

require "contracts"
include Contracts

class Animal; end
class Dog < Animal; end
class Cat < Animal; end

Contract Animal => Any
def foo x
p x
end

foo Dog.new # works, Dog is an Animal (checked with is_a?)
foo "hello" # fails, String is not an Animal
The check happens here.

If you want to do the opposite (i.e. accept Animal only, not Dog or Cat), use Exactly:

Contract Exactly[Animal] => Any
def foo x
p x
end

Reply to this email directly or view it on GitHub.

@naps62
Copy link
Contributor Author

naps62 commented Feb 23, 2016

@waterlink is right. following @egonSchiele's example, the use case for this would be something like:

Contract DescendantOf[Animal] => Animal
def animal_factory(x)
  x.new
end

dog = animal_factory(Dog)

without this, the best thing I can check (as far as I know) is just whether the argument is a Class:

Contract Class => Animal

@egonSchiele
Copy link
Owner

Oops, I misread! You're right, that is useful. Could you fix ci, then I'll merge it:

lib/contracts/builtin_contracts.rb:431:9: C: Style/RedundantReturn: Redundant return detected.
        return given_class.ancestors.include?(@parent_class)

@naps62
Copy link
Contributor Author

naps62 commented Feb 24, 2016

@egonSchiele ok great. The build is now fixed

This adds the ability to make assertions on the class hierarchy of a
class being passed as an argument. It should be useful when using
things like the Adapter Pattern or Dependency Injection.

Consider the following example:

```
class SlackClientAdapter < BaseClient
end

class HipchatClientAdapter < BaseClient
end

Contract C::DescendantOf[BaseClient] => BaseClient
def start_client(client_class)
  @client = client_class.new
end

start_client(SlackClientAdapter)
start_client(HipchatClientAdapter)
```

This is a demonstration of what I'm actually doing on a real app
(although the example itself is totally made up).

I want to be able to assert that the argument is not just a class, but a
descendant of `BaseClient` (this base client serves as an abstract class
this example. God, I feel like a Java developer all over again...)

I'm not sure if this is something that's already been proposed and
discarded, so I didn't take a lot of care in the organization of the
code I added.
Should this be accepted, I'll gladly make any required changes before
merging.
egonSchiele added a commit that referenced this pull request Feb 24, 2016
@egonSchiele egonSchiele merged commit 60ba358 into egonSchiele:master Feb 24, 2016
@egonSchiele
Copy link
Owner

Thank you for the PRs!

@@ -420,6 +420,30 @@ def inspect
attr_reader :options
end

# Use this for specifying contracts for class arguments
# Example: <tt>Descendant[ e: Range, f: Optional[Num] ]</tt>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this line be DescendantOf instead?

Copy link
Owner

Choose a reason for hiding this comment

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

You are correct @PikachuEXE. Fix is on master.

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.

4 participants