Skip to content

Added documentation to the init methods#96

Merged
raulriera merged 9 commits intomasterfrom
documentation-improvements
May 23, 2018
Merged

Added documentation to the init methods#96
raulriera merged 9 commits intomasterfrom
documentation-improvements

Conversation

@raulriera
Copy link
Copy Markdown
Contributor

@raulriera raulriera commented May 20, 2018

Added documentation to the init methods, also removed those parameters. We may need to apply different default values

Also moved the example files into several groups for easier navigation

if let name = name {
self.name = name
} else {
self.name = "FunctionalCollectionDataRenderAndDiff-\((#file as NSString).lastPathComponent):\(#line)"
Copy link
Copy Markdown
Contributor Author

@raulriera raulriera May 20, 2018

Choose a reason for hiding this comment

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

Suggestions to change these #file and #line to something else? Obviously outside of the init they won't work. But as parameters they are also pretty strange

"FunctionalCollectionDataRenderAndDiff-\(UUID().uuidString)" maybe? although not very useful for debugging.

Copy link
Copy Markdown
Contributor Author

@raulriera raulriera May 21, 2018

Choose a reason for hiding this comment

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

Thoughts @g-Off ? I used the UUID, but I don't know if the line and number where useful at some point

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UUID is even less useful. The file/line params indicate the creator of the instance of FunctionalTableData making it easier to trace back which instance may be generating a crash/exception

Copy link
Copy Markdown
Contributor Author

@raulriera raulriera May 21, 2018

Choose a reason for hiding this comment

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

In that case I think we should drop both then. Use the name only, there is no value in both

Copy link
Copy Markdown
Contributor

@g-Off g-Off May 21, 2018

Choose a reason for hiding this comment

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

Might be able to change this to name: String = "FunctionalCollectionDataRenderAndDiff-\(#file):\(#line)", which would give it a valid name all of the time and when one isn't provided would keep the file/line pairing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmmmm apparently that doesn't work either, they have to be in their own argument to get the reference from the caller. Otherwise this

public init(name: String = "FunctionalTableDataRenderAndDiff-\((#file as NSString).lastPathComponent):\(#line)")

becomes, this

(lldb) po name
"FunctionalTableDataRenderAndDiff-FunctionalTableData.swift:144"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Haha, sure why not, makes total sense 🤔 🙅‍♂️
Just the optional string name makes sense then. Although, why the desire to remove them? So they don't need to be documented?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No real reason apart from when I was documenting I went “hmmm what is this and why should someone care about these weird arguments”

I can put them back and not document them I gues. Best of both worlds?

@raulriera
Copy link
Copy Markdown
Contributor Author

Bonus round adds documentation for UIScrollView methods

@raulriera raulriera merged commit 74ed0ed into master May 23, 2018
@raulriera raulriera deleted the documentation-improvements branch May 23, 2018 16:18
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