eproxus / meck

A mocking library for Erlang
http://eproxus.github.io/meck
Apache License 2.0
811 stars 231 forks source link

Refactor meck into smaller functional modules #82

Closed horkhe closed 11 years ago

horkhe commented 11 years ago

@eproxus First draft is ready for you to check if I am going in the right direction. It is located under feature/mock_proc branch of my fork. There meck was split into four modules:

Type specs were mostly put in place.

All unit tests pass.

Dialyzer issues 3 reasonable warnings see ./dialyzer.ignore-warnings for details.

It really make no sence to look at diff for the changes are massive.

Hit me :)

horkhe commented 11 years ago

I have just force pushed changes to the same commit. Want be doing this anymore. All further changes will go as individual commits.

Besides the specified file module split up I fell erge to rename meck_mod to meck_code_util.

Anxiously waiting for you opinion on all these mess...

horkhe commented 11 years ago

Another big chunk of functionality was isolated in a module - meck_expect. As the name suggests it contains functions to create expectations and match them. It accumulated functionality formerly spread around meck, meck_proc, and meck_util.

None of the modules is longer then 500 lines of code now. And most of them are below 200. They all have rather slim API and most of the functions are private.

eproxus commented 11 years ago

Comments:

All in all, it looks good and I'd like to merge this as soon as the changes above are integrated.

horkhe commented 11 years ago

I will address these comments as soon as possible. How did yo draw this nice picture of deps by the way?

eproxus commented 11 years ago

My other project: https://github.com/eproxus/grapherl :smiley:

horkhe commented 11 years ago

Already fixed:

However I do not know how to resolve the cycle between meck_proc and meck_code_gen. meck_proc calls meck_code_gen:to_forms/2 only. And meck_code_gen calls meck_proc:get_ret_spec/3, meck_proc:add_history/5, and meck_proc:invalidate/1. Everything seems reasonable to me. Any concrete suggestions?

There are only two do_xxx functions and they both are implementations on a gen server API call. the reason for do_ prefix is that arity of internal functions is the same as of their API counterparts. Originally get_ret_spec was called get_expect and its implementation counterpart has the same name but different arity. I renamed it to better reflect the semantic of returned value. The other do_ function is do_delete_expect, originally API was delete and implementation was delete_expect. I renamed API yet again to make intention clearer. Please advise if this explanation seems reasonable to you and if not then please provide you suggestion.

Thanks a lot Adam for your comments. Great graph tool by the way.

horkhe commented 11 years ago

Since you you want to get this integrated sooner rather then latter I attached code to this issue.

horkhe commented 11 years ago

Travis CI status shows that the build against this pull request failed but it looks like a Travis issue itself.

eproxus commented 11 years ago

Let's leave the dependencies for later then. The do_...-naming sounds reasonable, so let's go with that for now.