O Xov, 29-04-2010 ás 16:40 +0200, Joaquim Rocha escribiu: > Hi folks, > > I am sending you the patch with Xabier's review. > > I would like to thank him here for the review work. > It got indeed better. > +static gchar * > +get_play_url_from_vimeo_xml (const gchar *xml, gint video_id) > +{ > + xmlDocPtr doc = xmlRecoverDoc ((xmlChar *) xml); > + xmlXPathContextPtr context = xmlXPathNewContext (doc); > + gchar *request_signature = get_node_text (context, > + "/xml/request_signature[1]"); > + gchar *signature_expires = get_node_text (context, > + "/xml/request_signature_expires[1]"); > + > + gchar *url = g_strdup_printf ("%s%d/%s/%s/?q=sd", > + VIMEO_VIDEO_PLAY_URL, > + video_id, > + request_signature, > + signature_expires); Something I didn't like the first and the second time, but I didn't complain before was this. I don't like declaring a variable by assigning the value of a function, and then using this value to declare another variable with another function call. It is like a subset of declaring variables where you want. I think this does not help to much to readability. What do other think about this? Br. -- Xabier Rodríguez Calvar Enxeñeiro en Informática IGALIA http://www.igalia.com
Attachment:
signature.asc
Description: Esta =?ISO-8859-1?Q?=E9?= unha parte de mensaxe asinada dixitalmente