From: Philip Withnall Date: Sun, 25 Dec 2011 10:48:14 +0000 (+0000) Subject: core: Warn on passing empty values to AbstractFieldDetails subclasses X-Git-Tag: FOLKS_0_6_7~53 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a99f47a196bee5a5ab258a010f7dcc173bbf7a42;p=platform%2Fupstream%2Ffolks.git core: Warn on passing empty values to AbstractFieldDetails subclasses There are no situations where passing an empty value (string or object) to an AbstractFieldDetails subclass makes sense. It just introduces potential bugs with mishandling empty strings (etc.) in code which consumes the AbstractFieldDetails. This commit adds warnings to all the AbstractFieldDetails subclasses’ constructors which check for empty inputs. We can't use assert()s here because that would be an API break. Helps: bgo#666540 --- diff --git a/folks/email-details.vala b/folks/email-details.vala index 81bc542..4ed3e15 100644 --- a/folks/email-details.vala +++ b/folks/email-details.vala @@ -37,12 +37,12 @@ public class Folks.EmailFieldDetails : AbstractFieldDetails /** * Create a new EmailFieldDetails. * - * @param value the value of the field + * @param value the value of the field, which should be a valid, non-empty + * e-mail address * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an * empty map of parameters. * - * * @return a new EmailFieldDetails * * @since 0.6.0 @@ -50,6 +50,11 @@ public class Folks.EmailFieldDetails : AbstractFieldDetails public EmailFieldDetails (string value, MultiMap? parameters = null) { + if (value == "") + { + warning ("Empty e-mail address passed to EmailFieldDetails."); + } + this.value = value; if (parameters != null) this.parameters = parameters; diff --git a/folks/im-details.vala b/folks/im-details.vala index 864894e..66becd5 100644 --- a/folks/im-details.vala +++ b/folks/im-details.vala @@ -46,7 +46,8 @@ public class Folks.ImFieldDetails : AbstractFieldDetails /** * Create a new ImFieldDetails. * - * @param value the value of the field + * @param value the value of the field, which should be a valid, non-empty + * IM address * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an * empty map of parameters. @@ -58,6 +59,11 @@ public class Folks.ImFieldDetails : AbstractFieldDetails public ImFieldDetails (string value, MultiMap? parameters = null) { + if (value == "") + { + warning ("Empty IM address passed to ImFieldDetails."); + } + this.value = value; if (parameters != null) this.parameters = parameters; diff --git a/folks/note-details.vala b/folks/note-details.vala index 838191a..cb0333f 100644 --- a/folks/note-details.vala +++ b/folks/note-details.vala @@ -56,7 +56,8 @@ public class Folks.NoteFieldDetails : AbstractFieldDetails /** * Create a new NoteFieldDetails. * - * @param value the value of the field + * @param value the value of the field, which should be a non-empty free-form + * UTF-8 string as entered by the user * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to a * empty map of parameters. @@ -69,6 +70,11 @@ public class Folks.NoteFieldDetails : AbstractFieldDetails MultiMap? parameters = null, string? uid = null) { + if (value == "") + { + warning ("Empty note passed to NoteFieldDetails."); + } + Object (value: value, id: uid, parameters: parameters); diff --git a/folks/phone-details.vala b/folks/phone-details.vala index f71f34f..bc525ff 100644 --- a/folks/phone-details.vala +++ b/folks/phone-details.vala @@ -54,7 +54,8 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails /** * Create a new PhoneFieldDetails. * - * @param value the value of the field + * @param value the value of the field, which should be a non-empty phone + * number (no particular format is mandated) * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to a * empty map of parameters. @@ -66,6 +67,11 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails public PhoneFieldDetails (string value, MultiMap? parameters = null) { + if (value == "") + { + warning ("Empty phone number passed to PhoneFieldDetails."); + } + Object (value: value, parameters: parameters); } diff --git a/folks/postal-address-details.vala b/folks/postal-address-details.vala index 6a6c8cc..b575a76 100644 --- a/folks/postal-address-details.vala +++ b/folks/postal-address-details.vala @@ -174,6 +174,26 @@ public class Folks.PostalAddress : Object } /** + * Whether none of the components is set. + * + * @return `true` if all the components are the empty string, `false` + * otherwise. + * + * @since UNRELEASED + */ + public bool is_empty () + { + return this.po_box == "" && + this.extension == "" && + this.street == "" && + this.locality == "" && + this.region == "" && + this.postal_code == "" && + this.country == "" && + this.address_format == ""; + } + + /** * Compare if two postal addresses are equal. Addresses are equal if all their * components are equal (where `null` compares equal only with `null`) and * they have the same set of types (or both have no types). @@ -246,7 +266,7 @@ public class Folks.PostalAddressFieldDetails : /** * Create a new PostalAddressFieldDetails. * - * @param value the value of the field + * @param value the value of the field, a non-empty {@link PostalAddress} * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an * empty map of parameters. @@ -259,6 +279,11 @@ public class Folks.PostalAddressFieldDetails : public PostalAddressFieldDetails (PostalAddress value, MultiMap? parameters = null) { + if (value.is_empty ()) + { + warning ("Empty postal address passed to PostalAddressFieldDetails."); + } + /* We keep id and value.uid synchronised in both directions. */ Object (value: value, parameters: parameters, diff --git a/folks/role-details.vala b/folks/role-details.vala index a910c92..b720148 100644 --- a/folks/role-details.vala +++ b/folks/role-details.vala @@ -98,6 +98,21 @@ public class Folks.Role : Object } /** + * Whether none of the components is set. + * + * @return `true` if all the components are the empty string, `false` + * otherwise. + * + * @since UNRELEASED + */ + public bool is_empty () + { + return this.organisation_name == "" && + this.title == "" && + this.role == ""; + } + + /** * Compare if two roles are equal. Roles are equal if their titles and * organisation names are equal. * @@ -165,7 +180,7 @@ public class Folks.RoleFieldDetails : AbstractFieldDetails /** * Create a new RoleFieldDetails. * - * @param value the {@link Role} of the field + * @param value the non-empty {@link Role} of the field * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an * empty map of parameters. @@ -177,6 +192,11 @@ public class Folks.RoleFieldDetails : AbstractFieldDetails public RoleFieldDetails (Role value, MultiMap? parameters = null) { + if (value.is_empty ()) + { + warning ("Empty role passed to RoleFieldDetails."); + } + /* We keep id and value.uid synchronised in both directions. */ Object (value: value, parameters: parameters, diff --git a/folks/url-details.vala b/folks/url-details.vala index 501588d..8b44eb3 100644 --- a/folks/url-details.vala +++ b/folks/url-details.vala @@ -74,7 +74,7 @@ public class Folks.UrlFieldDetails : AbstractFieldDetails /** * Create a new UrlFieldDetails. * - * @param value the value of the field + * @param value the value of the field, a non-empty URI * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to a * empty map of parameters. @@ -86,6 +86,11 @@ public class Folks.UrlFieldDetails : AbstractFieldDetails public UrlFieldDetails (string value, MultiMap? parameters = null) { + if (value == "") + { + warning ("Empty URI passed to UrlFieldDetails."); + } + Object (value: value, parameters: parameters); } diff --git a/folks/web-service-details.vala b/folks/web-service-details.vala index 4717183..c37bbb8 100644 --- a/folks/web-service-details.vala +++ b/folks/web-service-details.vala @@ -35,7 +35,7 @@ public class Folks.WebServiceFieldDetails : AbstractFieldDetails /** * Create a new WebServiceFieldDetails. * - * @param value the value of the field + * @param value the value of the field, a non-empty web service address * @param parameters initial parameters. See * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an * empty map of parameters. @@ -47,6 +47,12 @@ public class Folks.WebServiceFieldDetails : AbstractFieldDetails public WebServiceFieldDetails (string value, MultiMap? parameters = null) { + if (value == "") + { + warning ("Empty web service address passed to " + + "WebServiceFieldDetails."); + } + Object (value: value, parameters: parameters); }