[Kde-pim] KDE/kdepim/runtime

[Tobias Koenig]

On Tue, Jun 01, 2010 at 01:51:19AM +0200, Casey Link wrote:
> SVN commit 1133037 by link:
Hej Casey,

> Make the IMAP resource store its encryption setting as a string, rather
> then an integer representation of an enum.
> 
> THIS WILL BREAK YOUR CURRENT IMAP SETUPS!
Hmm, bad idea to commit something like that before a broader discussion...

> Short rationale: "SSL" is more understandable than "1"
So here the problem is that the content of the config file is not verbose enough...

> Longer rationale:  There are two enums used to
> represent encryption settings in kdepim, each with different values and
> orders. The result was hell whenever you had to reference the enum by
> integer value (like in javascript).
So this is a problem of the javascript -> c++ interface, not of the usage of enums
per se.

> Storing it as a String makes it easier and removes all ambiguity as to what the value represents.
It might make it more convenient, however you loose type checking by the compiler,
and every typo inside the strings will cause difficult to spot bugs.

> --- trunk/KDE/kdepim/runtime/resources/imap/imapresource.kcfg #1133036:1133037
> @@ -16,9 +16,9 @@
>      <entry name="UserName" type="String">
>        <label>Username</label>
>      </entry>
> -    <entry name="Safety" type="Int">
> +    <entry name="Safety" type="String">
We have the type 'Enum' for exactly this purpose, it will write out a string representation
to the config file and provides an enum for type safty on the C++ side.
That's exactly what you want in this case.

> +  switch ( m_ui->safeImapGroup->checkedId() ) {
> +  case KIMAP::LoginJob::Unencrypted :
> +    encryption = "None";
> +    break;
> +  case KIMAP::LoginJob::AnySslVersion:
> +    encryption = "SSL";
> +    break;
> +  case KIMAP::LoginJob::TlsV1:
> +    encryption = "STARTTLS";
> +    break;
> +  default:
> +    kFatal() << "Shouldn't happen";
> +  }
Code like this is error prone and difficult to maintain, should be done automatically as
kconfig_compiler does for Enum types.

> -  int i = Settings::self()->safety();
> +  QString safety = Settings::self()->safety();
> +  int i = 0;
> +  if( safety == "SSL" )
> +    i = KIMAP::LoginJob::AnySslVersion;
> +  else if ( safety == "STARTTLS" )
> +    i = KIMAP::LoginJob::TlsV1;
> +  else
> +    i = KIMAP::LoginJob::Unencrypted;
dito

Ciao,
Tobias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20100601/7a0c469b/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/