Added documentation and where clause for API `uniqued()` #32

Roshankumar350 · 2020-10-22T18:51:02Z

Problem:
* A quick look up for the API named uniqued() with example associated with it was missing.
* API named uniqued() while enumerating each element was checking an if condition.

Solution:
* API documentation added to address the same.
* where clause added for the same

Checklist

[ yes] I've added at least one test that validates that my change is working, if appropriate
[ yes] I've followed the code style of the rest of the project
[ yes] I've read the Contribution Guidelines
[ yes] I've updated the documentation if necessary

xwu · 2020-10-22T19:18:02Z

Good catch on the typo!

I've followed the code style of the rest of the project

Note that the code style involves indentations with two spaces, and also wrapping to 80 columns on each line. Sentences end with a period.

2. Wrapping to 80 columns on each line.

xwu · 2020-10-23T12:46:30Z

Sources/Algorithms/Unique.swift

+  /// order of the first occurrence of each unique element.
+  ///
+  /// In this example, `uniqued()` is used to include only names which are
+  /// unique


Thanks, the formatting is now mostly consistent. Make sure to include periods at the end of sentences, like here (also elsewhere). There is still another closing brace that is incorrectly indented. There are also several places where you have two consecutive spaces in the comment or the code; please make sure to remove them. There are also places where there are missing spaces after punctuation: please make sure to add them.

A more general comment here: I don’t think it adds anything to describe the example here by saying that uniqued makes the array unique. I think this line could be removed.

natecook1000

Thanks for filling out this documentation, @Roshankumar350! A few notes below for you.

Sources/Algorithms/Unique.swift

xwu · 2020-10-23T18:00:10Z

Sources/Algorithms/Unique.swift

  /// unique element.
+  ///
+  ///     let animals = ["dog", "pig", "cat", "ox", "dog","cat"]
+  ///     let uniqued = animals(on: { $0.first })


This does not compile.

Addressed and tested the same example on compiler.

xwu · 2020-10-23T18:00:53Z

Sources/Algorithms/Unique.swift

+  ///     let animals = ["dog", "pig", "cat", "ox", "dog","cat"]
+  ///     let uniqued = animals(on: { $0.first })
+  ///     print(uniqued)
+  ///     // Prints "["dog", "pig","cat", "ox"]"


Suggested change

/// // Prints "["dog", "pig","cat", "ox"]"

/// // Prints '["dog", "pig", "cat", "ox"]'

Although the output depends on what the projection chosen for this example is.

xwu · 2020-10-23T18:01:42Z

Sources/Algorithms/Unique.swift

+  ///
+  /// - Parameter projection: A projecting closure. `projection` accepts an
+  ///   element of this sequence as its parameter and returns a projected
+  ///   value of the same  type.


Suggested change

/// value of the same type.

/// value of the same type.

This description is true, but it’s also just describing the type signature in words. But what are the semantics expected of this closure? That should be documented here.

Added constraint of "Hashable" element

xwu · 2020-10-24T06:04:07Z

Sources/Algorithms/Unique.swift

@@ -42,14 +42,14 @@ extension Sequence {
  /// by the given projection), in the order of the first occurrence of each
  /// unique element.
  ///
-  ///     let animals = ["dog", "pig", "cat", "ox", "dog","cat"]
-  ///     let uniqued = animals(on: { $0.first })
+  ///     let animals = ["dog", "pig", "cat", "ox", "dog", "cat"]


This example doesn't really demonstrate the role of the 'projection' closure. If you want to stick to the projection $0.first, I'd suggest something like:

Suggested change

/// let animals = ["dog", "pig", "cat", "ox", "dog", "cat"]

/// let animals = ["dog", "pig", "cat", "ox", "cow", "owl"]

xwu · 2020-10-24T06:07:21Z

Sources/Algorithms/Unique.swift

  ///
  /// - Parameter projection: A projecting closure. `projection` accepts an
  ///   element of this sequence as its parameter and returns a projected
-  ///   value of the same  type.
+  ///   value of the same type having constraint `Hashable`.


The addition about 'Hashable' is useful, but the first part of this line is not correct: the projected value does not need to be of the same type.

But moreover, even when corrected, this statement is still describing the type of the parameter in words, which is not necessary and doesn't add to a user's understanding, assuming that they know how to read Swift type signatures. The challenge here is to teach the user why and how to use a projection function to determine uniqueness.

xwu · 2020-10-24T16:27:25Z

Sources/Algorithms/Unique.swift

+  /// - Parameter projection: A projecting closure. `projection` accepts an
+  ///   element of this sequence as its parameter which is having the type of
+  ///   projecting element and returns a projected value that may have the same
+  ///   type having constraint `Hashable`.


@natecook1000 Do you have suggestions of how to approach describing this parameter?

natecook1000

Getting close, @Roshankumar350!

natecook1000 · 2020-10-24T18:55:08Z

Sources/Algorithms/Unique.swift

+  /// - Parameter projection: A projecting closure. `projection` accepts an
+  ///   element of this sequence as its parameter which is having the type of
+  ///   projecting element and returns a projected value that may have the same
+  ///   type having constraint `Hashable`.


How about this?

Suggested change

/// - Parameter projection: A projecting closure. `projection` accepts an

/// element of this sequence as its parameter which is having the type of

/// projecting element and returns a projected value that may have the same

/// type having constraint `Hashable`.

/// - Parameter projection: A closure that transforms an element into the

/// value to use for uniqueness. If `projection` returns the same value

/// for two different elements, the second element will be excluded

/// from the resulting array.

@natecook1000 Thanks for forward thinking.

natecook1000 · 2020-10-24T18:55:11Z

Sources/Algorithms/Unique.swift

+  ///     let animals = ["dog", "pig", "cat", "ox", "cow", "owl"]
+  ///     let uniqued = animals.uniqued(on: {$0.first})


Let's give an introduction to this example, as well:

Suggested change

/// let animals = ["dog", "pig", "cat", "ox", "cow", "owl"]

/// let uniqued = animals.uniqued(on: {$0.first})

/// This example finds the elements of the `animals` array with unique

/// first characters:

///

/// let animals = ["dog", "pig", "cat", "ox", "cow", "owl"]

/// let uniqued = animals.uniqued(on: { $0.first })

xwu · 2020-10-25T13:57:15Z

This looks great!

Final step: looks like there's a conflict between this change and the main branch, and the revisions here are best as one commit instead of 18. So please squash and rebase this PR. Here's an introductory guide to doing that from the folks at Servo:

https://github.com/servo/servo/wiki/Beginner%27s-guide-to-rebasing-and-squashing

*Added documentation *Added documentation *consistant Indentation *1. Indentations with two spaces. *2. Wrapping to 80 columns on each line. *Indentations with two spaces for functions. *Example that demonstrates the uniqued(on:) method. *Addressing comment *Reverting lateral change. *Reverting lateral change *Addressing comment. *minor correction. *Addressing feedback. *Addressing feedback. *Addressing feedback. *Spacing *spacing *Addressing feedback.

…-algorithms into Readability Merged

xwu · 2020-10-26T11:40:41Z

Sources/Algorithms/Unique.swift

  /// unique element.
-  @inlinable


I don’t think this was rebased correctly. These annotations shouldn’t be deleted.

natecook1000 · 2020-10-26T22:12:33Z

Looks great, @Roshankumar350 — thanks for sticking with this! 🎉

Roshankumar350 added 4 commits October 19, 2020 22:14

Making code readable.

6969b96

Added documentation

91f56a5

Added documentation

955885f

consistant Indentation

5f50298

Roshankumar350 changed the title ~~Readability~~ Added documentation and where clause for API uniqued() Oct 22, 2020

Roshankumar350 added 2 commits October 23, 2020 11:46

1. Indentations with two spaces.

725d68b

2. Wrapping to 80 columns on each line.

Indentations with two spaces for functions.

2270423

xwu reviewed Oct 23, 2020

View reviewed changes

natecook1000 requested changes Oct 23, 2020

View reviewed changes

Sources/Algorithms/Unique.swift Outdated Show resolved Hide resolved

Sources/Algorithms/Unique.swift Outdated Show resolved Hide resolved

Sources/Algorithms/Unique.swift Outdated Show resolved Hide resolved

Sources/Algorithms/Unique.swift Outdated Show resolved Hide resolved

Roshankumar350 added 6 commits October 23, 2020 21:19

Example that demonstrates the uniqued(on:) method.

38bd0ab

Addressing comment

5779714

Reverting lateral change.

7d60b3c

Reverting lateral change

8ed7a50

Addressing comment.

8c4ad09

minor correction.

38f6253

xwu reviewed Oct 23, 2020

View reviewed changes

Roshankumar350 added 2 commits October 24, 2020 10:27

Addressing feedback.

51e0ecf

Addressing feedback.

56d50c9

xwu reviewed Oct 24, 2020

View reviewed changes

Roshankumar350 added 3 commits October 24, 2020 21:41

Addressing feedback.

723f02e

Spacing

511fffa

spacing

c2ee769

xwu reviewed Oct 24, 2020

View reviewed changes

natecook1000 reviewed Oct 24, 2020

View reviewed changes

Addressing feedback.

14e24eb

Roshankumar350 added 3 commits October 26, 2020 12:52

Merge branch 'main' into Readability

ee7ae20

Merge branch 'Readability' of https://github.com/Roshankumar350/swift…

f1bd757

…-algorithms into Readability Merged

xwu reviewed Oct 26, 2020

View reviewed changes

Addressing feedback.

2bee012

natecook1000 merged commit 66ec968 into apple:main Oct 26, 2020

Roshankumar350 deleted the Readability branch October 27, 2020 05:14

	/// // Prints "["dog", "pig","cat", "ox"]"
	/// // Prints '["dog", "pig", "cat", "ox"]'

	/// let animals = ["dog", "pig", "cat", "ox", "dog", "cat"]
	/// let animals = ["dog", "pig", "cat", "ox", "cow", "owl"]

		/// let animals = ["dog", "pig", "cat", "ox", "cow", "owl"]
		/// let uniqued = animals.uniqued(on: {$0.first})

Added documentation and where clause for API uniqued() #32

Added documentation and where clause for API uniqued() #32

Uh oh!

Conversation

Roshankumar350 commented Oct 22, 2020

Checklist

Uh oh!

xwu commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xwu Oct 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natecook1000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xwu Oct 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natecook1000 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xwu commented Oct 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natecook1000 commented Oct 26, 2020

Uh oh!

Uh oh!

Added documentation and where clause for API `uniqued()` #32

Added documentation and where clause for API `uniqued()` #32

xwu commented Oct 22, 2020 •

edited

Loading

xwu Oct 23, 2020 •

edited

Loading

xwu Oct 24, 2020 •

edited

Loading