Hi Henrik,
Yeah I already made the all changes except one (session in constructor).
Whether use new function like authenticateUser() or in execute() one ? . I
prefer new function.
I will commit the new changes soon. Right now I am exploring some cities of
India :).
And yeah re-factoring wins , thanks to your support :)
--
Mohyt
On Sat, Jun 16, 2012 at 3:08 AM, Henrik Ingo <henrik.i...@avoinelama.fi>wrote:
> On Fri, Jun 15, 2012 at 12:44 PM, Henrik Ingo <henrik.i...@avoinelama.fi>
> wrote:
> > I didn't look at HTTPHandler yet. Will do that separately. Looks
> > really good now!
>
> http_handler.cc
>
>
> handleRequest():
>
> I don't know if this is official or just me, but for local variables,
> don't use the leading underscore:
>
> const char* _input_query;
>
> Just do this:
>
> const char* input_query;
>
> When reading, I expect that member variables have the leading
> underscore so this was confusing.
>
>
> This check should be done also for the POST case:
>
> if ( _input_query == NULL || strcmp(_input_query, "") == 0 )
> {
> _input_query = "{}";
> }
>
> You could delete that one and then do this after the if/else block:
>
> if ( _query == NULL || strcmp(_query, "") == 0 )
> {
> _query = "{}";
> }
>
>
> Please use the same text for both error messages. Also, in the below
> sentences "specified" is grammatically correct. Also, use "must" when
> something is required (http://www.ietf.org/rfc/rfc2119.txt)
>
> _json_out["error_message"]= "Table isn't specify in URI query string";
> _http_response_text = "Table should be specify in URI query string.";
>
> So in other words:
>
> _json_out["error_message"]= _http_response_text = "table must
> be specified in URI query string.";
>
> (non-capital t in table is intentional.)
>
>
> Again, Drizzle custom is to return false on success and true on error
> (yeah, it's crazy...):
>
> return false;
> }
>
> return true;
> }
>
> Did you switch all the other ones I pointed out in previous review?
>
> Same in validateJson() :
>
> return retval;
> }
>
> Change to "return !retval"; (Ok, at this point it's starting to not
> make sense anymore. But consistency is important...)
>
> Btw. Notice now how we have achieved a state where most of the code
> that used to be copy-pasted around the 3 separate GET/POST/DELETE
> functions is now actually shared code, and you have only small
> branches for handling POST or GET separately in HTTPHandler and
> SQLToJsonGenerator, and of course the separate branches in
> SQLGenerator.
>
> This means that in json_server.cc you can just put the code to call
> HTTPHandler and DBAccess directly in
> process_api02_json_req()
>
> ...and you can delete
> process_api02_json_get_req()
> process_api02_json_post_req()
> process_api02_json_put_req()
> process_api02_json_delete_req()
>
> ...which will be a major re-factoring WIN!
>
> henrik
> --
> henrik.i...@avoinelama.fi
> +358-40-8211286 skype: henrik.ingo irc: hingo
> www.openlife.cc
>
> My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559
>
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : drizzle-discuss@lists.launchpad.net
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
