Skip to content
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.

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

Add some cool extensions to Array and Dictionary #95

Merged
merged 3 commits into from Mar 12, 2016

Conversation

DulMephistos
Copy link
Contributor

I'd love to use this extension at the project that I've been using at work but in order to do that I need those cool extensions to be added. They're pretty useful and some were extracted from ExSwift project.

@Esqarrouth
Copy link
Owner

Thanks for the PR. Are you using all of these functions?

@DulMephistos
Copy link
Contributor Author

Yeah, we do. We currently have an augmented version of EXSwift running on NFL Mobile but since they stopped maintaining their project I want to move to something else that's currently being maintained and is always up to date. We could have this code running only internally but I'm sure more people might have the same needs as we do and that way we can help others and build cool extensions on top of your project.

@Esqarrouth
Copy link
Owner

Sorry for late reply, was attending a hackathon and since this was a big addition wanted to test everything.

Thank you for sharing your extensions with the community, some of these would be really useful for a lot of people. Some seem to be more useful and intuitive for developers from certain background like python, js. So I am not completely sure on adding them to the project, I'll share my thoughts on some of them, feel free to bring up counter arguments.

@Esqarrouth
Copy link
Owner

Array stuff:

myArray.sortUsing()

Is this in anyway different from:

public func sort(@noescape isOrderedBefore: (Self.Generator.Element, Self.Generator.Element) -> Bool) -> [Self.Generator.Element]
public mutating func sortInPlace(@noescape isOrderedBefore: (Self.Generator.Element, Self.Generator.Element) -> Bool)

myArray.all()

I think that myArray.testAll() would be a better naming convention

myArray.each { (object: String) -> () in
    print(object)
}

vs

for object in myArray {
    print(object)
}

I do not believe each() makes it easier for neither writing or reading, its use case is good in js and ruby, but I think the 2nd method is a much better way for swift. Let me know if I am missing something

var myArray = ["first","second","third","fourth","fourth"]
myArray.mapFilter { (str) -> (V)? in
    if str == "first" {
        return nil
    }
    return str
}

Error: Use of undeclared type V

myArray.pop()
print(myArray)

vs

myArray.popLast()
print(myArray)

I think apple added popLast() after the extension of pop() was created. Now pop() seems unnecessary.

myArray.shift()
print(myArray)

vs

myArray.removeFirst()
print(myArray)

I think this is the same scenario. People coming from another language might prefer to use shift() instead of removeFirst(), but this makes it worse for people who doesn't know that language since removeFirst() is more intuitive. Even if the whole team comes from shift() using languages changing the code to something like this would be better:

mutating func shift() -> Element? {
    return self.removeFirst()
}

myArray.unshift()

vs

myArray.insert("hey", atIndex: 0)

Same issue as above

func ==<T: Equatable>(lhs: [T]?, rhs: [T]?) -> Bool {

vs

public func ==<Element : Equatable>(lhs: [Element], rhs: [Element]) -> Bool

When I use == it automatically uses the Swift standard at the bottom. What is the use case on the top one?

@Esqarrouth
Copy link
Owner

Dictionary stuff:

Couldn't make dict.union() work

dict.toArray { (h, v) -> V in
    return v
}

I get the use of undeclared type V error

dict.mapValues()

Same as above

dict.each { (str, int) -> () in
    print(str)
}

vs

dict.forEach { (str, int) -> () in
    print(str)
}

These both do the same thing.

dict.each()

vs

dict.forEach()

vs

for (key, value) in dict {
    print(key)
    print(value)
}

I think each, like in array is not much of a use

dict.all()

Maybe name it testAll()

@DulMephistos
Copy link
Contributor Author

Yeah, I'm sorry. Some stuff was definite implemented by swift after we started using these extension and they're no longer necessary and some others I have arguments on why they're useful. So I'll elaborate one by one.

Array Stuff:
1 - You're right

2 - Agreed

3 - I'd argue that, as everything in the extension, it makes some code more readable and easier to write. Specially when you want to execute one thing only on each object. Consider this scenario:

You want to remove all subviews from a view.

for view in subviews {
   view.removeFromSuperview()

vs

subviews.each { $0.removeFromSuperview() }

4 - MapFilter is intended to 'map' each object to another type and then filter out the nil objects.

var myArray = ["first","second","third","fourth","fourth"]
myArray.mapFilter { (str) -> (Int)? in
    if str == "first" {
        return nil
    }
    return myArray.indexOf(str) + 1
}

Also very useful if you have an array of optionals Element? and you want to make it an array of non-optionals.

let optionalArray: [String?] = ["first","second",nil,"fourth","fourth", nil, "six"]
let nonOptionalArray = optionalArray.mapFilter { $0 }

5 - You're right.

6 - You're right.

7 - It's less typing. Maybe we could have something like insertAtFirst? If not, I totally understand.

8 - The one that I'm proposing works with optional arrays types. Try to compile this:

let array: [String]? = ["first","second", "fourth", "fourth", "six"]
let aArray2: [String]? = ["first","second", "fourth", "fourth", "six"]

optionalArray == optionalArray2

@DulMephistos
Copy link
Contributor Author

Dictionary stuff:

I actually forgot to put some custom operators that we use as well to take advantage of union, intersection and difference. So we can write things like dict1 & dict2 - dict3

1 - The union functions takes one or more dictionaries and updates itself with values from those dictionaries for the same keys. Unifies multiple similar dictionaries into one.

2 -
It takes a map function, so in this case you need to specify the Type you want it to be mapped to

dict.toArray { (key, value) -> String in
    return `MAPPED_TO_SOME_STRING`
}

3 - Same as above, but it'll work on values only.

4 - Agreed, they have forEach now.

5 - forEach works.

6 - Agreed.

@DulMephistos
Copy link
Contributor Author

I'm going to update my pull request with the changes we agreed. Thanks a lot for testing everything.

@DulMephistos
Copy link
Contributor Author

Let me know how you feel about everything after the changes. Thank you!

@Esqarrouth
Copy link
Owner

Array:

3- Yes you suggestion looks good and easy to read. However it is very hard to write, autocomplete isn't helping too much here, so is the compiler, I couldn't get it to work:

Type of expression is ambiguous without more context

I could argue that if its difficult for me, a lot of people can't use it.

4- I see, seems useful.

7- insertAtFirst() makes sense

8- Cool, missed that

@Esqarrouth
Copy link
Owner

Dictionary:

1-

var dict: Dictionary = ["first": 1, "second": 2, "third": 3]
var anotherdict: Dictionary = ["fourth": 4]
dict.union(anotherdict) //Error: Ambigous use of 'union'

2-

So essentially these are the same things?

var dict: Dictionary = ["first": 1, "second": 2, "third": 3]

let arry = dict.toArray { (key, value) -> String in
    return key
}

var arry2 = [String]()
for item in dict {
    arry2.append(item.0)
}

Esqarrouth added a commit that referenced this pull request Mar 12, 2016
Add some cool extensions to Array and Dictionary
@Esqarrouth Esqarrouth merged commit 7b44dd1 into Esqarrouth:master Mar 12, 2016
@DulMephistos
Copy link
Contributor Author

Yeah, some of it is more dedicated to declarative programming, which we use a lot in our code. We avoid doing regular loops, and manual transformations inside those loops.
By using each, map, toArray, etc.. we can chain some operations and the intend becomes so clear in the code, maybe not how it'll get done, but that's the beauty of declarative programming.

Imagine that you have this structure:

class Player {
    let name: String

    init(name: String)
}

class TopPlayer {
    let player: Player
    let teamAbbr: String    

    init(player: Player, teamAbbr: String)
}

Then you receive from back-end a dictionary with all top players by team, something like this:

let topPlayersByTeam: [String: String] = ["NE": "Tom Brady", "DEN": "Peyton Manning"...]

And you're asked to transform all that to an array of TopPlayer objects. Using declarative programming you could do:

let topPlayers = topPlayersByTeam
    .mapValues { (_, value) in Player(name: value) }
    .toArray { (key, value) in TopPlayer(player: value, teamAbbr: key) }

And that's the beauty, now you have an array of TopPlayer without doing any for loops yourself :)

The union, intersection and difference on the dictionary stuff, I imagine are the most difficult to wrap your head around... we use this concept because we deal with Updatable streams of data and we constantly need to do a kind of differential on those dictionaries to know exactly what have changed and merge it how we want. But please let me know if you think this is totally unnecessary on your extensions and I can make those live inside our project and not yours, I want you to feel good about your own project, of course :)

One last thing, I noticed that you have some unit tests and that I did not write any, my apologies. I'll write some tests for those new functions.

Have a great weekend 😎

@Esqarrouth
Copy link
Owner

.mapValues().toArray() chaining seems cool.

Need Array.each() to be fixed or removed, doesn't work now.

Union, intersection, difference seems intuitive and simple. Only problem is union doesn't work 😁

Unit tests would be nice, also adding examples of the new funcs in readme is needed too.

You have a great weekend too 😎

@Esqarrouth
Copy link
Owner

Btw stuff should have been public

487d41f
798de71

@DulMephistos
Copy link
Contributor Author

You're right.. my bad. I'll change our project this week and make it finally use your extension.

I'm also planning to write the unit tests for these functions this week, as soon as time allows.

thanks for the update

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.

None yet

2 participants