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

Make passthrough/1' and func/1 into a ret_spec` #90

Closed
wants to merge 2 commits into from
Closed

Make passthrough/1' and func/1 into a ret_spec` #90

wants to merge 2 commits into from

Conversation

horkhe
Copy link
Contributor

@horkhe horkhe commented Nov 19, 2012

introduced two new ret_spec() kinds that are meck:passthrough/0 and meck:func/1 that make the following possible:

%% When
meck:expect(meck_test_module, c, [{[1, 2],     meck:passthrough()},
                                  {['_', '_'], meck:func(fun(A, B) -> {B, A} end)}]),
%% Then
?assertEqual({1, 2}, meck_test_module:c(1, 2)),
?assertEqual({3, 1}, meck_test_module:c(1, 3)),

That comes pretty handy given that you can specify them within meck:seq/1 and meck:loop/1

Besides all ret_spec() aware code was moved to a module of the same name.

@horkhe
Copy link
Contributor Author

horkhe commented Nov 19, 2012

@eproxus can I expect comments today, or I just can go to sleep? :)

@eproxus
Copy link
Owner

eproxus commented Nov 19, 2012

I don't have time to review all of it today anyway, so let's keep in touch later. 😃

@horkhe
Copy link
Contributor Author

horkhe commented Nov 19, 2012

sure thing, but please do not disappear for too long :-)

@horkhe
Copy link
Contributor Author

horkhe commented Nov 19, 2012

I hope we can release 0.8 with all that plus matchers before the end of this year.

@eproxus
Copy link
Owner

eproxus commented Nov 19, 2012

Sounds reasonable. I'll review this as soon as I can, promise. 🙇

%%%============================================================================

%%% @private
%%% @doc Provides expectation processing functions.
Copy link
Owner

Choose a reason for hiding this comment

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

What's the difference between meck_ret_spec and meck_expect? Could they both be merged into the same module perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

meck_ret_spec specifies how to calculate a value that is returned by an expectation. meck_expect represents an expectation that is defined by a list of meck_args_spec/meck_ret_spec pairs where every pair is akin to a function clause/ So these are two different abstraction though the latter using the former as a building block.

In general I create these different modules not because I just like to have lots of files :-), but because I prefer to have distinct data abstractions with clean API and hidden implementation. I believe that greatly improves readability of the code..

@eproxus
Copy link
Owner

eproxus commented Nov 25, 2012

How about removing meck:func/1 and letting a fun directly have the same effect? Then, if one wanted to have a fun as return value they would have to use meck:val/1 around the fun?

@horkhe
Copy link
Contributor Author

horkhe commented Nov 25, 2012

We certainly can make funs serve as a body of a function but, that would break backwards compatibility. and make another exception in ret spec definition. now any X that you specify derectly is treated as meck:val(X) no exceptions. With you propousal we will have exception from that rule. Which means that users of the meck would need to remember that exception.

So it is you call, but I would recommend to stick with meck:fun. less exceptions from general rules we have the better I think.

@horkhe
Copy link
Contributor Author

horkhe commented Dec 8, 2012

@eproxus Hi Adam, it has been awhile since I provided my answers. So what do you think?

@eproxus
Copy link
Owner

eproxus commented Dec 12, 2012

I usually prefer conciseness in API's, especially for stuff that is often used. I think wrapping funs in meck:val(F) is an okay trade off.

@horkhe
Copy link
Contributor Author

horkhe commented Dec 12, 2012

But what about backwards compatibility? Now if you specify ret-spec as
value (including funs) it is used literally.

Maxim Vladimirsky
Sent from my Nexus
On Dec 12, 2012 11:28 PM, "Adam Lindberg" notifications@github.com wrote:

I usually prefer conciseness in API's, especially for stuff that is often
used. I think wrapping funs in meck:val(F) is an okay trade off.


Reply to this email directly or view it on GitHubhttps://github.com//pull/90#issuecomment-11305110.

@horkhe
Copy link
Contributor Author

horkhe commented Dec 12, 2012

Ok, I will make that change and let you know when it is ready. Probably
tomorrow.

Maxim Vladimirsky
Sent from my Nexus
On Dec 12, 2012 11:28 PM, "Adam Lindberg" notifications@github.com wrote:

I usually prefer conciseness in API's, especially for stuff that is often
used. I think wrapping funs in meck:val(F) is an okay trade off.


Reply to this email directly or view it on GitHubhttps://github.com//pull/90#issuecomment-11305110.

@eproxus
Copy link
Owner

eproxus commented Dec 13, 2012

Hmm, so far this has never made it into a proper 0.X release, so I wouldn't worry about backwards compatibility.

@horkhe
Copy link
Contributor Author

horkhe commented Dec 13, 2012

Ok. It also occurred to me that it does resemble more the former
meck:expect(Mod, Func, Fun) syntax. So I will rework processing of fun's
according to you suggestion.

On Thu, Dec 13, 2012 at 2:51 PM, Adam Lindberg notifications@github.comwrote:

Hmm, so far this has never made it into a proper 0.X release, so I
wouldn't worry about backwards compatibility.


Reply to this email directly or view it on GitHubhttps://github.com//pull/90#issuecomment-11330211.

@horkhe
Copy link
Contributor Author

horkhe commented Dec 13, 2012

Closed as a not extendable pull request. It continues as #91.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants