Skip to content

add suffix:while method #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 9, 2021
Merged

add suffix:while method #65

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9ef531f
add suffix while method
michiboo Jan 20, 2021
1acc213
add test for suffix
michiboo Jan 20, 2021
813ea79
add more test for suffix Tests
michiboo Jan 22, 2021
08e424e
add doc on suffix method
michiboo Jan 23, 2021
60bad3b
Update Sources/Algorithms/Suffix.swift
michiboo Jan 23, 2021
2f8f798
Update Sources/Algorithms/Suffix.swift
michiboo Jan 24, 2021
f9a4f79
Update Sources/Algorithms/Suffix.swift
michiboo Jan 24, 2021
3b51107
Fix suffix tests
michiboo Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions Sources/Algorithms/Suffix.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Algorithms open source project
//
// Copyright (c) 2020 Apple Inc. and the Swift project authors
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Copyright (c) 2020 Apple Inc. and the Swift project authors
// Copyright (c) 2021 Apple Inc. and the Swift project authors

// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
//
//===----------------------------------------------------------------------===//

//===----------------------------------------------------------------------===//
// Suffix(while:)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Suffix(while:)
// suffix(while:)

//===----------------------------------------------------------------------===//

extension BidirectionalCollection {
@inlinable
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation should be added. Mirroring the documentation for prefix(while:) would be a good start.

public __consuming func Suffix(while predicate: (Element) throws -> Bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public __consuming func Suffix(while predicate: (Element) throws -> Bool
public __consuming func suffix(
while predicate: (Element) throws -> Bool

) rethrows -> [Element] {
Copy link
Contributor

@mdznr mdznr Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can this return SubSequence instead?

prefix(while:) on Collection returns a SubSequence.[1]

  1. https://github.com/apple/swift/blob/main/stdlib/public/core/Collection.swift#L1328

var result = ContiguousArray<Element>()
self.reverse()
Copy link
Contributor

Choose a reason for hiding this comment

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

Consistent indentation

Suggested change
var result = ContiguousArray<Element>()
self.reverse()
var result = ContiguousArray<Element>()
self.reverse()

Copy link
Contributor

@mdznr mdznr Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don’t believe reverse() can be used here (won’t compile), because it is a mutating function that reverses the elements of the collection in-place. This function, suffix(while:), isn’t mutating (and should remain that way), so it can’t use self.reverse() in its implementation.

It seems that you might have meant to use the non-mutating version of that function, reversed(), which returns a reversed collection:

    for element in self.reversed() {

However, this comment likely won’t be applicable when changing the function to return SubSequence instead.

for element in self {
guard try predicate(element) else {
break
}
result.append(element)
}
result.reverse()
return Array(result)
Copy link
Contributor

@mdznr mdznr Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This double-reverse operation and converting to Array shouldn’t be necessary. The last index of the collection can be retrieved with endIndex, then walked backwards using index(before:).

This would be similar to the implementation of prefix(while:)[1], but a little bit trickier. Since endIndex is beyond the subscript-able range of a collection, the index has to be moved backwards before using it to subscript the collection, and ensuring it doesn’t go before the first element (start index). In SuffixTests.swift, I made some comments about test cases that will be important for ensuring that these cases are accounted for.

  1. https://github.com/apple/swift/blob/main/stdlib/public/core/Collection.swift#L1330

}
}
20 changes: 20 additions & 0 deletions Tests/SwiftAlgorithmsTests/SuffixTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Algorithms open source project
//
// Copyright (c) 2020 Apple Inc. and the Swift project authors
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Copyright (c) 2020 Apple Inc. and the Swift project authors
// Copyright (c) 2021 Apple Inc. and the Swift project authors

// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
//
//===----------------------------------------------------------------------===//

import XCTest
import Algorithms

final class SuffixTests: XCTestCase {

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

func testSuffix() {
let a = 0...10
XCTAssertEqualSequences(a.suffix(while: { $0 > 5 }), (6...10))
}