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