Comparison conversion issues

[arlomedia]

I've just started using Swiftify this week and have converted about 20 files so far. I've noticed a few odd conversions that come up consistently, having to do with comparisons. Since these seem similar I thought they could share the same thread:

Input: if (![[appData objectForKey:@"scheme"] isEqualToString:@""]) Output: if (!(appData["scheme"] == "")) Expected: if (appData["scheme"] != "") Comment: code will run the same way, but is less readable

Input: ((enabled)&&(controlBar.startButton)) Output: ((enabled)&&(controlBar.startButton)) != nil Expected: ((enabled)&&(controlBar.startButton != nil)) Comment: enabled is a boolean and startButton is an object that I'm checking for nil; I think the code will run the same way, but the meaning is less clear

Input: if (self.appDelegate.mainViewController.splitView) Output: if appDelegate.mainViewController.splitView != nil Expected: if appDelegate.mainViewController.splitView Comment: splitView is a boolean, so there's no need to add the nil comparison, and I think that changes the way the code runs because a boolean set to false is not nil and will evaluate to true here

When several comparisons are combined into one conditional check and these changes are applied, the code can get quite hard to read, and I'm afraid of losing an exclamation mark or otherwise flipping a condition when I clean it up.

[alex-swiftify]

Thanks, @arlomedia for reporting! While there is no universal solution to all fix such cases, we will prioritize fixing the cases that you have mentioned, at least the first two.

The third case is a bit tricky. The converter is adding a nil comparison because:

• related files with declarations of appDelegate, mainViewController and splitView properties were not included in the conversion;
• known declaratons of the splitView property, i.e. NSSplitViewController.splitView declare it as an object (i.e. NSSplitView) rather than a boolean.

If you include related files that reference all mentioned properties in the same conversion, you are likely to get the correct result without the nil comparison.

We are planning to add a feature to the Advanced Project Converter to be able to specify external header files that should be considered.

However, this will be more likely included in the Offline version, since including this in the Cloud version would require sending many header files with each conversion.

[arlomedia]

Thanks for the explanation. Treating splitView as an object makes sense when you point out that an iOS framework has an object with that name.

Here's another example of the first case:

Input: if (![self.interactivePopGestureRecognizer.delegate isEqual:self]) Output: if !(interactivePopGestureRecognizer?.delegate == self)

The current conversion is totally logical, and works, but it would be nice when converting a negated isEqual or isEqualToString function to a simple x != y instead of the easily misunderstandable !(x == y).

[alex-swiftify]

Thanks @arlomedia , the suggestion is completely understandable and we'll look at implementing it as you suggested. This seems not trivial but possible.

[alex-swiftify]

@arlomedia

1. Fixed the translation of isEqual: andisEqualToString:.

Input:

if (![self.interactivePopGestureRecognizer.delegate isEqualToString:self]) {
}

Output:

if interactivePopGestureRecognizer?.delegate != self {
}

Input:

if (![[appData objectForKey:@"scheme"] isEqualToString:@""]) {
}

Output:

if appData["scheme"] != "" {
}

This will be available after the next converter update.

2. I have tried and I cannot get the output that you mentioned. a) Your exact code sample: http://swiftify.me/n077iw b) Same but including class & method declarations: http://swiftify.me/pxd1jk c) Same including declarations of controlBar and startButton - includes correct nil comparison: http://swiftify.me/f76eb4

Let me know the exact way to reproduce the output that you have got:

((enabled)&&(controlBar.startButton)) != nil

3. Got your confirmation on this, so no changes here:

Treating splitView as an object makes sense when you point out that an iOS framework has an object with that name.

[arlomedia]

For case 2, I can't find the original example of this (I renamed the variables in these examples for simplicity), but I can post a more complete example if I run into it again.

For case 3, I've been seeing this with other booleans, not just when the names are the same as objects in the SDK. But maybe the issue is the same even without a naming overlap -- if a boolean is defined in another class that isn't part of the conversion, the converter doesn't know it's a boolean and treats it like a generic object. In that case, your conclusion about the need to somehow reference related files during the conversion would be the same.

[arlomedia]

Here's another example of case 3. In this example, I'm not checking a boolean directly, but checking a function that returns a boolean value:

Input: if ([self.appDelegate.deviceLinking screenIsBroadcasting]) { Output: if appDelegate.deviceLinking.screenIsBroadcasting() != nil { Expected: if appDelegate.deviceLinking.screenIsBroadcasting() {

Input: if (![self.appDelegate.deviceLinking screenIsBroadcasting]) { Output: if appDelegate.deviceLinking.screenIsBroadcasting() == nil { Expected: if (!appDelegate.deviceLinking.screenIsBroadcasting()) {

I understand that if the converter doesn't have access to the definition for screenIsBroadcasting, it doesn't know whether to treat its return value as a boolean or a generic object, so it defaults to treating it as a generic object and compares it against nil. But would it be better to default to treating it as a boolean, and provide the expected output shown above? With that setup, if the return value were not a boolean, the compiler would show an error and the developer could fix it by adding the != nil. That's safer than the current setup, where if the return value is a boolean, the compiler doesn't show an error but the functionality of the code changes: the condition will evaluate true whether the return value is true or false, because neither true nor false are nil.

In short, when you have if (unknownType), assuming unknownType is a boolean might be the safest option.

[arlomedia]

I've run into some examples similar to case 2 today, but they're converting differently in the web-based converter compared to the Mac app:

Input: if ((self.tempoSendCustomMidi)&&([self.tempoSendCustomMidiValues count])) Output on web: if tempoSendCustomMidi && (tempoSendCustomMidiValues.count()) Output in app: if tempoSendCustomMidi && (tempoSendCustomMidiValues.count) != 0

Input: if (([self.appDelegate getPrefsValue:@"tempoDownbeatSound"])&&(![[self.appDelegate getPrefsValue:@"tempoDownbeatSound"] isEqualToString:@""])) Output on web: if (appDelegate.getPrefsValue("tempoDownbeatSound")) && (!(appDelegate.getPrefsValue("tempoDownbeatSound") == "")) Output in app: if (appDelegate.getPrefsValue("tempoDownbeatSound")) != nil && (!(appDelegate.getPrefsValue("tempoDownbeatSound") == ""))

In both examples, the website output seems fine, but the app output confusingly adds a "!=" comparison outside the parentheses. I think this could be improved by either 1) not adding the "!=" comparison, like the website, 2) removing the parentheses, which would be consistent with other parentheses stripping, or 3) if the "!=" comparison and the parentheses are both needed, put the "!=" comparison inside the parentheses.

[alex-swiftify]

@arlomedia

For case 3, I've been seeing this with other booleans, not just when the names are the same as objects in the SDK. But maybe the issue is the same even without a naming overlap -- if a boolean is defined in another class that isn't part of the conversion, the converter doesn't know it's a boolean and treats it like a generic object. In that case, your conclusion about the need to somehow reference related files during the conversion would be the same.

Understandable. We are planning on the option to reference external headers when converting a file. This is something more likely to appear first in the Offline version than the Cloud one since it requires sending many files to the converter during each file conversion.

[alex-swiftify]

Here's another example of case 3. In this example, I'm not checking a boolean directly, but checking a function that returns a boolean value:

Input: if ([self.appDelegate.deviceLinking screenIsBroadcasting]) { Output: if appDelegate.deviceLinking.screenIsBroadcasting() != nil { Expected: if appDelegate.deviceLinking.screenIsBroadcasting() {

Input: if (![self.appDelegate.deviceLinking screenIsBroadcasting]) { Output: if appDelegate.deviceLinking.screenIsBroadcasting() == nil { Expected: if (!appDelegate.deviceLinking.screenIsBroadcasting()) {

I understand that if the converter doesn't have access to the definition for screenIsBroadcasting, it doesn't know whether to treat its return value as a boolean or a generic object, so it defaults to treating it as a generic object and compares it against nil. But would it be better to default to treating it as a boolean, and provide the expected output shown above? With that setup, if the return value were not a boolean, the compiler would show an error and the developer could fix it by adding the != nil. That's safer than the current setup, where if the return value is a boolean, the compiler doesn't show an error but the functionality of the code changes: the condition will evaluate true whether the return value is true or false, because neither true nor false are nil.

In short, when you have if (unknownType), assuming unknownType is a boolean might be the safest option.

Can you post a complete sample to reproduce this? I believe that you could get such an output, but feeding your sample to our online converter gives a correct output without nil comparison: http://swiftify.me/nu8twy

[alex-swiftify]

I've run into some examples similar to case 2 today, but they're converting differently in the web-based converter compared to the Mac app:

Input: if ((self.tempoSendCustomMidi)&&([self.tempoSendCustomMidiValues count])) Output on web: if tempoSendCustomMidi && (tempoSendCustomMidiValues.count()) Output in app: if tempoSendCustomMidi && (tempoSendCustomMidiValues.count) != 0

Input: if (([self.appDelegate getPrefsValue:@"tempoDownbeatSound"])&&(![[self.appDelegate getPrefsValue:@"tempoDownbeatSound"] isEqualToString:@""])) Output on web: if (appDelegate.getPrefsValue("tempoDownbeatSound")) && (!(appDelegate.getPrefsValue("tempoDownbeatSound") == "")) Output in app: if (appDelegate.getPrefsValue("tempoDownbeatSound")) != nil && (!(appDelegate.getPrefsValue("tempoDownbeatSound") == ""))

I couldn't easily reproduce this, too. Online code snippet: http://swiftify.me/nu8twy Sample converted with the Advanced Project Converter: test.zip In my case, converting online and in the app produced the same results.

Please post a complete file that you are trying to convert, indicate how you are converting it (i.e. Advanced Project Converter, Xcode, or Finder extension), and check if resetting all conversion options to defaults fixes the output.

[arlomedia]

Thanks for looking into it. I just emailed you the complete file that these examples came from.

[alex-swiftify]

@arlomedia Reviewed your files and addressed at least the 3 cases that you have suggested in the email.

Case 2. The issue was that the if condition's type was inferred as an optional unknown type (denoted as UnknownType? internally in our converter). For example, for this expression:

if (([self.appDelegate getPrefsValue:@"tempoDownbeatSound"])&&(![[self.appDelegate getPrefsValue:@"tempoDownbeatSound"] isEqualToString:@""])) {
}

the type of (appDelegate?.getPrefsValue("tempoDownbeatSound")) was inferred as UnknownType? because the getPrefsValue() method was not included in this particular conversion.

Previously, our converter inserted a nil comparison to this expression. Now we resort to leaving this expression "as is".

This is now translated as follows

if (appDelegate?.getPrefsValue("tempoDownbeatSound")) && (appDelegate?.getPrefsValue("tempoDownbeatSound") != "") {
}

Note that the "correct" translation should be unwrapping the boolean like this: appDelegate?.getPrefsValue("tempoDownbeatSound") ?? false

but I've decided that we should not assume boolean when the return type of getPrefsValue() is unknown.

Similarly,

    if ([self.appDelegate.deviceLinking screenIsBroadcasting]) {
    }

is now translated as follows:

        if appDelegate?.deviceLinking.screenIsBroadcasting() {
        }

Also, fixed unwrapping of integer properties like count. Now the following:

if ((self.tempoSendCustomMidi)&&([self.tempoSendCustomMidiValues count])) {
    [self.appDelegate startMIDI];
}

is translated to:

if tempoSendCustomMidi && (tempoSendCustomMidiValues?.count ?? 0) != 0 {
    appDelegate?.startMIDI()
}

This will be available after the next converter update within less than a week.