Cache sections and numberOfSection

[JosephDuffy]

These properties are accessed quite frequently and can be cached with a little extra processing.

This is another finding from screens that have a large number of composed sections.

This and composed-swift/ComposedUI#15 seem to be the main areas of performance issues, although that doesn't mean that once these are working and merged we won't uncover more 😅

Thanks to @bill201207 for their contributions to this change.

[shaps80]

LGTM but is there a test(s) we can include to ensure this works correctly?

[shaps80]

👍

[JosephDuffy]

LGTM but is there a test(s) we can include to ensure this works correctly?

There are existing tests that run as part of the PR and cover most of the uses but with a couple of extra tests the coverage for the file could probably be ~98%.

Adding Codecov to get an idea of coverage change would be useful in the future, in case the existing tests don't cover something added.

[shaps80]

Don't you use Xcode for that while coding? I find it helps me cover the majority early on.

[JosephDuffy]

Don't you use Xcode for that while coding? I find it helps me cover the majority early on.

It's not enabled in the shared scheme but I will commit a change to enable it as part of the PR.

Xcode's coverage is useful, but it doesn't cover all cases:

• I change something in FileA that causes the coverage of (unchanged) FileB to be reduced
  • I would never normally spot this in Xcode (unless the coverage was previously ~100%)
  • This often points to a failing of the tests for FileB, but I still find it useful
• I make a "small change" and push without noticing the last of test coverage
  • In this sense it's like a "last line of defence"
• As a viewer I don't need to pull the branch and run the tests to see if the new changes have been covered

Percentage covered is still not a great metric of code quality, but I have found that having coverage as part of PRs has caught issues earlier.

Codecov is free and it's quite easy to setup but I don't have the permission to add it to the organisation. I think it's well worth it though.

[shaps80]

It's not enabled in the shared scheme but I will commit a change to enable it as part of the PR.
Good shout!

• This often points to a failing of the tests for FileB, but I still find it useful
  Agreed

• As a viewer I don't need to pull the branch and run the tests to see if the new changes have been covered
  Great point, more important in OSS for sure!

Percentage covered is still not a great metric of code quality, but I have found that having coverage as part of PRs has caught issues earlier.
Agreed, that's why I like to see line-by-line in Xcode whether I've tested a specific code path at all. Not as an overall metric, but its a good start I think.

Codecov is free and it's quite easy to setup but I don't have the permission to add it to the organisation. I think it's well worth it though.
Agreed, we're using in at Thirdfort as well. I think I read the other day you can ever get Xcode integration so it 'feed's the Codecov data?

–

Just to clarify, I definitely wasn't suggesting Xcode as a replacement. I just meant while coding, I personally find it really helpful to see those per-line numbers, so I know if I've 100% missed a code-path. Especially when there's a few conditions in a specific function for example with complicated paths.

On a side note, this approach also sometimes helps identify less testable code as well.

[JosephDuffy]

I think I read the other day you can ever get Xcode integration so it 'feed's the Codecov data?

They have a bash uploader but when running the tests using swift test (as opposed to xcodebuild) there are some extra hoops to jump through.

We can copy the config from https://github.com/JosephDuffy/Persist/blob/master/.github/workflows/tests.yml and it should work 👍

[JosephDuffy]

@shaps80 I think this should be good to go now? Once merged I'll release 1.1.1.

Codecov can be enabled in another PR.

[shaps80]

Yep, looks great thanks! Merge away