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