Rethink if we should initialize NSString and collection objects with non-optional empty values

Description

`SKSTableView` project, SKSTableView.swift:

Produces the following (when SkipIfStatementsForNonOptionalVariables option is set to true):

Problem(s):
1) When the `SkipIfStatementsForNonOptionalVariables` option is set to true, our logic skips the entire body of the property accessor since the backing field variable is always non-optional;
2) When the `SkipIfStatementsForNonOptionalVariables` option is not set, our code should rather check if `_expandableCells` is an empty dictionary than compare it with nil.

Once this is addressed, we may rethink whether `SkipIfStatementsForNonOptionalVariables` option can be set to true by default.

Environment

None

Activity

Show:
Alex Petuschak
September 13, 2019, 2:14 PM
Edited

What’s your opinion about our “old” approach to initialize string and collection types with non-optional default values, like here? http://swiftify.me/5kvpv0

Initially, this was implemented in order to reduce the number of optionals and unwrapping operators.

However, for cases like backing fields (this task’s code sample), this looks completely wrong.

Here are my thoughts about possible options:

1) Making this option configurable is possible but maybe error-prone to support both “optional“ and “non-optional“ scenarios at the same time.

I still think this is the next thing to do (nilinitializers follow the original Obj-C code; non-nil initializers result in less unwrapping operators, especially for arrays/dictionaries).

Would you suggest using nilor empty string/collection initializers by default?

2) The best possible way I can think of is analyzing the code to see if a variable is ever used in a context that allows nilvalue.

This may require huge efforts to support and test, since the same variable declaration would be changed to optional or not, depending on the context.

3) Always use nil rather than empty string/collection initializers for backing fields (see this task’s code sample).

Keep non-optional string/collection initializers for most other cases.

Ivan Kh
September 16, 2019, 4:06 PM
Edited

My rating is:

  1. Analyzing the code

  2. Always use nil rather than empty string/collection initializers (you can look at number of errors in Automated testing for optional and non-optional and choose one with smaller errors count)

  3. Making this option configurable (I don’t like options, it's better if it work as is).

 

However, for cases like backing fields (this task’s code sample), this looks completely wrong.

Absolutely agree with you, any option above better than this solution with ”#if false"

Assignee

Alex Petuschak

Reporter

Alex Petuschak

Labels

None

Git Branch Name

Feature/OptionalVariableInitializers

GitHub Issue

None

Components

Fix versions

Priority

High
Configure