Esqarrouth/EZSwiftExtensions

Consider refactoring to protocol extensions

DanielAsher opened this issue · 15 comments

Hi,

as a follow on from #386 I wanted to know if any consideration has been given to refactoring the framework to use protocol extensions. This could allow per-file limited imports of specific features.

As an example, currently Array.get is defined as:

extension Array {

    ///EZSE: Get a sub array from range of index
    public func get(at range: ClosedRange<Int>) -> Array {
        var subArray = Array()
        let lowerBound = range.lowerBound > 0 ? range.lowerBound : 0
        let upperBound = range.upperBound > self.count - 1 ? self.count - 1 : range.upperBound
        for index in lowerBound...upperBound {
            subArray.append(self[index])
        }
        return subArray
    }
    /// EZSE: Gets the object at the specified index, if it exists.
    public func get(at index: Int) -> Element? {
        guard index >= 0 && index < count else { return nil }
        return self[index]
    }

This seems to be imported into a file without any explicit import EZSwiftExtensions declaration.

Could this be refactored to:

public protocol EZArray : Collection { 
}

extension EZArray where Self.Index == Int, Self.IndexDistance == Int  {

    public func get(at index: Int) -> Self.Iterator.Element? {
        guard index >= 0 && index < count else { return nil }
        return self[index]
    }
    
    public func get(at range: ClosedRange<Int>) -> Array<Self.Iterator.Element> {
        var subArray = Array<Self.Iterator.Element>()
        let lowerBound = range.lowerBound > 0 ? range.lowerBound : 0
        let upperBound = range.upperBound > self.count - 1 ? self.count - 1 : range.upperBound
        for index in lowerBound...upperBound {
            subArray.append(self[index])
        }
        return subArray
    }
}

which would allow a file to either

  1. import protocol EZSwiftExtensions.EZArray (limited import)
  2. import EZSwiftExtensions (full import)

I'm not sure this would solve the problem (I don't really understand the subtleties of swift's import mechanism) or why a file would import EZSwiftExtensions without an explicit import EZSwiftExtensions declaration.

Thanks again!

@DanielAsher I get your concern. I believe the namespace pollution comes from adding the library as a target dependency in your code. Alternatively if you use something like pods, IIRC they pull the source down and there is a compilation conflict.

My only concern is breaking the existing userspace. @goktugyil @lfarah @piv199 - What do you guys think. If we take this up, we would have to dedicate an entire release for this.

This would require heavy refactoring, will change the behaviour for all users. And I'm not sure if will actually work for all cases.

We try to differentiate extensions from standard libraries and make it clear with the docs that its an extension to minimize the problem.

Can we at least find a quick fix for #386's forEach?

I presume the fix for this would be to remove this

/// EZSE: Iterates on each element of the array.
@available(*, deprecated: 1.6, renamed: "forEach(_:)")
public func each(_ call: (Element) -> Void) {
    forEach(call)
}

If yes I will remove it ASAP.

But if it's deprecated, why is @DanielAsher getting an error?

I dont know why he is still getting an error.

#389

@Khalian - that for the quick PR and merge. I believe that deprecated extension methods still override standard library implementations.

Could you remove the following deprecated extension method from the library:

    @available(*, deprecated: 1.7, renamed: "forEachEnumerated(_:)")
    public func forEach(_ call: @escaping (Int, Element) -> Void) {
        forEachEnumerated(call)
    }

this should complete and close the issue.

Thanks!

Ok will do

One more! Please remove deprecated removeAll
So sorry didn't report this earlier.

Removed removeAll in PR.

It looks like Swift 3.1 will greatly simplify refactoring to finely-grained protocol extensions with https://bugs.swift.org/browse/SR-1009 being implement. Constrained extensions allow same-type constraints so refactoring StringExtension.swift simply requires:

public protocol EZString {}

extension String : EZString {}

extension EZString where Self == String {
    /// EZSE: Character count
    public var length: Int {
        return self.characters.count
    }
/// ...
}

Hey @DanielAsher, that looks really cool!

Can we close this, @Khalian?

@lfarah We could but @DanielAsher has requested we deprecate : https://github.com/goktugyil/EZSwiftExtensions/blob/928d825dcc5002f468d8cfc0b94b98a5910f160c/Sources/ArrayExtensions.swift#L135

We can have a quick discussion on the merits and demerits of the same.