Re: [PATCH 2/2] Add handling for more than one address in vCard

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Rafal,

On Wed, Sep 01, 2010, Rafał Michalski wrote:
> +static struct phonebook_address *find_address(GSList *addresses,
> +					const char *address, int type)
> +{
> +	GSList *l = addresses;
> +
> +	for (; l; l = l->next) {

Please avoid initialization upon declaration of variables when possible.
The first part of the for-statement is where you should usually
initialize the iterator, i.e. in this case do the l = addresses there.

> +static gboolean address_fields_present(const char *address)
> +{
> +	gchar **address_fields = g_strsplit(address, ";",
> +							ADDR_FIELD_AMOUNT);
> +	int i = 0;
> +
> +	for (; i < ADDR_FIELD_AMOUNT; ++i) {

Same here for i. Additionally you could rename address_fields to simply
fields since the function is short and the context pretty obvious.
You'll also avoid the split line that way (another thing I wouldn't
object to is to move the initialization separate from the variable
declaration).

> -static gboolean address_fields_present(struct phonebook_contact *contact)
> +static gboolean address_fields_present(const char *address)
>  {
> -	gchar **address_fields = g_strsplit(contact->address, ";",
> +	gchar **address_fields = g_strsplit(address, ";",
>  							ADDR_FIELD_AMOUNT);
>  	int i = 0;

Hmm, it seems you have the same function in vcard.c and
phonebook-tracker.c? Why not remove the phonebook-tracker.c one and
export the vcard.c version through vcard.h?

Johan