Skip to content

Add Recursive Depth check#8

Open
john-twigg-ck wants to merge 6 commits intock/swift-5from
ck/RecursiveDepth
Open

Add Recursive Depth check#8
john-twigg-ck wants to merge 6 commits intock/swift-5from
ck/RecursiveDepth

Conversation

@john-twigg-ck
Copy link

@john-twigg-ck john-twigg-ck commented Jan 10, 2020

We had our call stack blowing up regularly.
Add a new Dip Error and reporting to help identify recursive errors.

It can print out a report like the following:

Recursive Depth exceeded: Cycle Found: [DipTests.RecusiveTests.(unknown context at $106ad07f8).ServiceA, DipTests.RecusiveTests.(unknown context at $106ad0758).ServiceB, DipTests.RecusiveTests.(unknown context at $106ad06b8).ServiceC]

The Max is defaulted to 100. Can be configured statically.

Comment on lines +160 to +161
var cycleSubStack = [Any.Type]()
var hitCount = [String:Int]()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styling:

var cycleSubStack: [Any.Type] = []
var hitCount: [String:Int] = []

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?? I'm not aware of us having a preference on this one.
Its not like we do
let foo: Foo = Foo() either...

Copy link

@DulMephistos DulMephistos Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it's confusing but we do reference two style guides in our docs unfortunately. One of them is more kept up to date and more comprehensive. That one says that for empty arrays / dictionaries, we should inform the type and initialize with the short form, which is either [] or [:]

ref: https://github.com/ck-private/mob_ios/blob/stable/Documentation/Styleguide/STYLEGUIDE.md#type-annotation-for-empty-arrays-and-dictionaries

@DulMephistos
Copy link

All minor stuff .. great change John, this will help a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants