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

Sorting by filename #78

Merged
merged 1 commit into from
Dec 2, 2017
Merged

Conversation

DmIvanov
Copy link

@DmIvanov DmIvanov commented Dec 2, 2017

Not sure if you agree with all the changes, but I'll try to explane them.

The original idea was to add standard sorting by filename to the table. I found it might be quite convenient to work file by file.

There were already several variants of data source in the ViewController (dataSource, filteredData, perFunctionTimes, perFileTimes), so adding one more wasn't an option. So I decided to incapsulate all the processing over the data array to a separate class (ViewControllerDataSource), store only one original array of items and apply all the aggregating, filtering and sorting dynamically if necessary. In my opinion it makes code and the logic cleaner and easily lets extend processing functionality (other operations under the data array if necessary).

Unfortunately I had to convert CompileMeasure to an objC class for being able to easily apply an array of sort descriptors. I couldn't find easy and elegant way to apply several sorting functions to swift array without a lot of pain (didn't want to implement something like http://chris.eidhof.nl/post/sort-descriptors-in-swift/). I thought that inheriting the model from NSObject and double casting the array (to NSArray and back to swift array) is not such a big deal considering the ability to use good old sort descriptors (which are built in to NSTableView)

All the basic functionality of ViewControllerDataSource is covered with tests.

- incapsulating ViewControllerDataSource
- some unit tests
@RobertGummesson
Copy link
Owner

Nice work! I like it. We can live with the objc bits so let's bring this in.

@RobertGummesson RobertGummesson merged commit e860691 into RobertGummesson:master Dec 2, 2017
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