Exception implementation for C#, Delphi, etc.

I've some comments/suggestions regarding exception mechanisms within the Turboactivate implementation for C#, Delphi, etc.:

1) It would be nice to have an option to disable all (or eventually also parts of the) turbo activate exceptions (or two variants of function calls). I see two reasons for this: a) If exceptions are used it might be more easy to track this down (especially in managed code) and understand how licensing is implemented in the software. b) From software design view, I think it is not necessary to raise an exception if e.g. on optional feature is not there. This is not an error because it can be expected and shouldn't be treated as such.

2) The exceptions are all implemented in a flat hierarchy. This is not very convenient if all exceptions should be caught in one place and only some of the need special treatment (all others will be handled equally). I think it would be good to have e.g. GeneralTurboActivateException as base and then derive all other exceptions from it.

3) Many exception messages can be shown e.g. within an GUI to the user without any security concerns, but there is one, which is named GUIDMismatchException and seems problematic to me, because its message contains the product GUID ("The version GUID \"" + TurboActivate.VersionGUID + "\" doesn't match that of the product details file. Make sure you set the GUID using TurboActivate.VersionGUID."). It could be replayed by e.g. "The product details file "TurboActivate.dat" does not match the product." like I've now done within our product.

Best

We had a big discussion about whether we should use exceptions or just have a thin wrapper over the C interface of TurboActivate. We ended up using exceptions because it fits with the nature of the languages (and the performance loss is so small as to be non-existent).

2) The exceptions are all implemented in a flat hierarchy. This is not very convenient if all exceptions should be caught in one place and only some of the need special treatment (all others will be handled equally). I think it would be good to have e.g. GeneralTurboActivateException as base and then derive all other exceptions from it.

You're right -- the exceptions should share a common base 1 level higher than "Exception". We'll fix this for TA 3.2 or 3.3. Thanks for suggesting that.

3) Many exception messages can be shown e.g. within an GUI to the user without any security concerns, but there is one, which is named GUIDMismatchException and seems problematic to me, because its message contains the product GUID ("The version GUID \"" + TurboActivate.VersionGUID + "\" doesn't match that of the product details file. Make sure you set the GUID using TurboActivate.VersionGUID."). It could be replayed by e.g. "The product details file "TurboActivate.dat" does not match the product." like I've now done within our product.

We used to have something like what you're suggesting. The problem is that it's confusing for new users just trying out LimeLM.

Plus, in production you should eat that exception no matter what wording we use. Does that make sense?

2) The exceptions are all implemented in a flat hierarchy. This is not very convenient if all exceptions should be caught in one place and only some of the need special treatment (all others will be handled equally). I think it would be good to have e.g. GeneralTurboActivateException as base and then derive all other exceptions from it.

You're right -- the exceptions should share a common base 1 level higher than "Exception". We'll fix this for TA 3.2 or 3.3. Thanks for suggesting that.

Regarding this, in my reimplementation of the TurboActivate pas i made all excpetion derived from a common base that was exposing also the error code number. Like that i have been able to make generic excepts without needing to use the boring is operator or inheritsfrom method and in case i don't want to show the original message i can still show " a generic error happened. Code: %d" to let my user return me the code.

And I'm am agreeing that in some cases it can be helpful an helping method to check if there is or not a feature, instead of trying to read it and generate an error that then must be trapped. In delphi there are some example like StrToInt and TryStrToInt or in the TDictionary<TKey,TValue> there is a trygetvalue.

And I'm am agreeing that in some cases it can be helpful an helping method to check if there is or not a feature, instead of trying to read it and generate an error that then must be trapped. In delphi there are some example like StrToInt and TryStrToInt or in the TDictionary<TKey,TValue> there is a trygetvalue.

Ok, we'll change the behavior of GetFeatureValue() to return null if the feature doesn't exist (instead of throwing an exception).

And I'm am agreeing that in some cases it can be helpful an helping method to check if there is or not a feature, instead of trying to read it and generate an error that then must be trapped. In delphi there are some example like StrToInt and TryStrToInt or in the TDictionary<TKey,TValue> there is a trygetvalue.

Ok, we'll change the behavior of GetFeatureValue() to return null if the feature doesn't exist (instead of throwing an exception).

But this will break the compatibility with past code. If I would be in you I would add another method, like the example of StrToInt, that will do the same things of the GetFeatureValue without generating an error.

The current GetFeatureValue is:

class function GetFeatureValue(featureName: LPCWSTR): LPWSTR;

An example of this new method (to add to the current one, not to replace) could be:

class function TryGetFeatureValue(featureName: LPCWSTR; out Value: LPWSTR):Boolean;

If reading the feature will succeed the result will be true, otherwise false.

Right now a feature value can never be null (by design). But I understand if someone just drops in the newest TurboActivate.cs or TurboActivateUnit.pas they'll want the behavior to be unchanged.

How would an override of the existing GetFeatureValue() work for you? Someting like

class function GetFeatureValue(featureName: LPCWSTR): LPWSTR;


class function GetFeatureValue(featureName: LPCWSTR; throwException: boolean): LPWSTR;

So if you call GetFeatureValue(...) like you normally would you'll get an exception, otherwise if you pass in a "false" to throwException you'll get a null value when the feature doesn't exist.

Does that sound good?

I think having a call for GetFeatureValue that does not throw an exception will be very good.I'd suggest having an overload for GetFeatureValue where a default value can be passed as second argument. If the feature is not available, the method will return the passed default instead of null.

For the GUIDMismatchException, would it be possible to remove just the GUID value? ("The version GUID doesn't match that of the product details file. Make sure you set the GUID using TurboActivate.VersionGUID." instead of "The version GUID \"" + TurboActivate.VersionGUID + "\" doesn't match that of the product details file. Make sure you set the GUID using TurboActivate.VersionGUID.")?

Best

I'd suggest having an overload for GetFeatureValue where a default value can be passed as second argument. If the feature is not available, the method will return the passed default instead of null.

You're right -- that's a better idea. Thanks for suggesting it.

For the GUIDMismatchException, would it be possible to remove just the GUID value? ("The version GUID doesn't match that of the product details file. Make sure you set the GUID using TurboActivate.VersionGUID." instead of "The version GUID \"" + TurboActivate.VersionGUID + "\" doesn't match that of the product details file. Make sure you set the GUID using TurboActivate.VersionGUID.")?

That's what it used to be, but it confused new users to LimeLM & TurboActivate. Maybe we can use an #ifdef to use the more descriptive error message only on debug builds.

Having the GUID in the error message only in debug would be the best solution I think.

One more thing regarding the GetFeatureValue: Having the overload with the default value it would also be possible to create overloads for the other data types: Integer and Date. What do you think?

Off-Topic: PDetsFromPath method seems to be missing in Delphi implementation.

One more thing regarding the GetFeatureValue: Having the overload with the default value it would also be possible to create overloads for the other data types: Integer and Date. What do you think?

I don't see the benefit of this because they'd have to be converted to strings before they were returned.

Off-Topic: PDetsFromPath method seems to be missing in Delphi implementation.

You're right -- we'll fix that for TA 3.2

My wording probably was not exact enough. The type of the return value should match the type of the default value. Here is an example for what I'm thinking of (C# example):public static string GetFeatureValue(string featureName, string defaultValue);public static int GetFeatureValue(string featureName, int defaultValue);public static DateTime GetFeatureValue(string featureName, DateTime defaultValue);

This way the user wouldn't have to do the conversions himself (but could do if using the first version).

We'll think on that. We might add it to TA 3.3.