-
Notifications
You must be signed in to change notification settings - Fork 44
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
Refactored and prepared erlog for production use #13
base: master
Are you sure you want to change the base?
Conversation
comtihon
commented
Jun 21, 2014
- got rid of passing functions everywhere. Maybe it is little bit faster, but code is hard to maintain. Refactored erlog gen_server, made erlog logic more OTP like: now there is one function to call - execute and all work is done through handle_call. Server state (db and others) is kept in #state of gen_server.
- refactored erlog_int, got rid of defining code (as from my point of view it is not good way), move storage functions from erlog_int to callback interface, made callback interface realisation for ets and dict (dict not working properly for now).
- created erlog_memory - gen_server, which works with memory (dict, ets, database, and others implementation). There is no need to pass db param as argument everywhere in code. It is stored is #state of erlog gen_server. (But I don't remove passing db in some places, as there are lots of them).
- moved all modules to folders
- moved functions from huge (500+ line of codes) modules to smaller one
- erlog_int -> erlog_memory, erlog_core
There is a lot of stuff here to go through! I will pull it locally and check it out. This may take some time as I am summer holidays and my programming has become more relaxed. Also I have to prepare a talk + demo for OSCON so please don't expect a speedy response, but I will get back to you on this. I noticed you have worked on the interface. I have just written up some independent thoughts on this to the erlog mailing list. It is a Google group at: https://groups.google.com/forum/?fromgroups=#!forum/erlog Please join if you wish. Traffic is low and I am trying to get more potential users and have it as a place for discussions. |
I joined and posted my answer. |
Question, why does execute return a binary take this code: fail_test() ->
{ok, PID} = erlog:start_link(),
?assertEqual(fail,erlog:execute(PID, "fail.")),
true. erlog:execute/2 returns the binary <<"No">>, might it make more sense to return it as an atom 'no' or better yet 'false'? It definitely seems more like other erlang functions to me |
It doesn't matter, I suppose, but if you prefer - it can be true/false. |
Well having it be bool() makes it work a lot better with many erlang things. |
I don't think it should return a boolean or a binary. I think proving goal should return the variable bindings if it succeeds so a boolean wouldn't fit, something like {true,Bindings} is just bad. I know we have a a few times in the libraries but that doesn't mean we should imitate it. And is shouldn't be false | NotBoolean. That is why I chose {succeed,Bindings} | fail. |
What @rvirding said, the return state should be what it was {succeed,[binding()]}|fail, also take a look at the quickcheck properties I created and lets see if we can make those work. |
So, what are the plans for merging? |
As your PR does some restructuring and renaming I would first like a discussion on how we want to structure the erlog distribution. I will start one on the mailing-list |
Conflicts: src/core/erlog.erl