Code Refactoring
Opened this issue · 6 comments
Hey @modocache, created this issue for us to discuss the refactoring changes. Figured it would be easier than trying to find a closed pull request.
Had a look at RestKit, will definitely be using that for v2, very similar to RestSharp which is a c# library that I use for the windows phone version of the app which I'll hopefully be releasing as soon as I can afford a dev license, although depending on how things go that might jump straight to v2. Code is available in my [Coderwall-WP7(https://github.com/OiNutter/Coderwall-WP7]) repo if you want to take a look.
Gonna sit down over the weekend and take a proper look at the tests and stuff you've added, especially as that's stuff I'd like to be using on my new app as well.
Also if there's any more appropriate Objective-C ways of doing stuff than what I've done in the app feel free to let me know, Like I said it's my first app so I've likely not done some stuff in the most efficient way.
Basically any suggestions about improving the code structure chuck them down in this issue. Same goes for anybody else reading this, feel free to weigh in with any constructive feedback and suggestions.
Cheers
Will
Awesome, thanks.
The tests aren't much to look at right now, I'm hoping to add to them as I go about polishing the code in general.
I sent you a pull request which pretty much exemplifies how I'd like to refactor some parts of the app. I won't change any of the logic, but I do think it would help to make some stuff neater--using a single syntax for method definitions--(NSString *)saySomething; seems to be more common over (NSString*) saySomething;, so I went with the former--and organizing method definitions using #pragma markers.
Let me know if you have any criticisms/suggestions with any of the pulls I'm going to be issuing today.
Issued another pull request, this time for MasterViewController. You have a lot of code that's repeated throughout the application, so I consolidated it within a category. I also added some integration tests, although they're pretty fragile.
I was hoping to issue these commits atomically, but it seems like that might be difficult to do after this one, considering some of these refactors have a pretty far reach. My requests from this point on will likely build on one another.
Feedback on any of these refactors would be appreciated, whenever you get the chance.
Have merged both of those in. Had a quick scan through the diffs and don't have any issues with any of it. More than happy for the code to be tweaked to follow more commonly used Objective-C style guidelines. Categories look interesting, came across those briefly while working on my current app. Looks like I might need to take a better look as they may be better for what I need to do in it. I'm going to have a look see if I can link this to Travis now you've added integration tests, would be good for people to see that it's passing its tests.
Nope, doesn't look like travis supports Objective C yet.
Yeah, you could write iOS integration tests in JavaScript, but personally I prefer KIF. You could run the KIF and unit tests via Jenkins, sometime soon I'll try and look into how we could set up a web hook to run the tests on commit/pull request.
Nah, happy to stick with KIF and see how that goes. Was just thinking it would be nice to have something like travis doing automatic integration testing on push. I've used Travis before and liked the fact that I didn't have to set up my own server to run the tests. Was also interested in the forthcoming 'testing of pull requests' feature. Will keep an eye out and hope their adding support for ObjC/iOS. In the meantime I may look into what's involved in setting up a Jenkins instance for us to use.