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

Internal code architecture improvement #53

Open
ilya-murzinov opened this issue Sep 23, 2014 · 13 comments
Open

Internal code architecture improvement #53

ilya-murzinov opened this issue Sep 23, 2014 · 13 comments

Comments

@ilya-murzinov
Copy link
Collaborator

@artem-zinnatullin, you've mentioned once in #48 that WAIL has bad architecture. So may be it's time to improve it using tools you listed?
Or it's not the part of your plan for WAIL? :)

@artem-zinnatullin
Copy link
Owner

Yep, if you want to do this — I'll be happy to help you :)

So, let's do this?

@ilya-murzinov
Copy link
Collaborator Author

Alright! :)
Can you please draw up a plan how we should do this?)

@artem-zinnatullin
Copy link
Owner

We should start with easiest things, to feel how it's good to use great tools and throw away boilerplate and bad code :)

So, suggested plan:

  1. ButterKnife

Please read carefully all documentation, you'll find that ButterKnife uses Annotations processing to generate boilerplate code at compile time, you just need to mark required fields (views) and methods (such as onClickListeners) with annotations and then place call to ButterKnife.inject(this, view) (or another overload of inject() after view inflating, for example in onViewCreated() and ButterKnife will use generated code to do injects.

I suggest you to refactor all fragments and replace findViewById() and setting of onClickListeners code with ButterKnife

Here is the first task: #54

If you got any questions -> feel free to ask me here or via email :)

Waiting a pull request from you! Hope you'll love ButterKnife and later you'll use it in your work!

P.S. please carefully test all parts refactored with ButterKnife, sometimes views should be marked as optional, because if view not present at the time when ButterKnife.inject() called, it will throw an exception

I'll keep this issue as plan, after resolving first issue, I'll add next part of BIG REFACTORING PLAN!

@ilya-murzinov
Copy link
Collaborator Author

Okay, seems nice to me :)

@artem-zinnatullin
Copy link
Owner

You'll love it :)

@artem-zinnatullin
Copy link
Owner

Next step: Otto it's an implementation of EventBus pattern.

Here is the documentation http://square.github.io/otto/, please read it carefully and think about places, where you can use it :)

I'd suggest to use it instead of BroadcastReceivers for notifying the Ui, for example for notifying MainFragment about scrobbling track and so on.

Btw, you can create custom Event classes with final fields as event args (it's a good practice), also, notice that by default, Otto will throw exception if you'll send event from non Ui thread, there are several workarounds for that, you can ask me if you want :)

It's not Android-only pattern, you can use EventBuses in any Java apps, you'll love it!

Here is the issue: #60

@artem-zinnatullin
Copy link
Owner

Also, will be great to switch to Retrofit + OkHttp + Gson for communicating with Last.fm API

Retrofit is such an awesome library, you just need to write a Java Interface with annotated methods and it will generate an implementation which'll be able to do Http requests to any endpoint

Please read documentation carefully and try to switch one of the requests to the Retrofit (it uses Gson by default to parse JSON, so response type must be marked with @serializable Gson annotations, if you'll got issues or questions — just ask me :)

The issue: #66

@ilya-murzinov
Copy link
Collaborator Author

Sounds nice, I'll give it a try after fixing and merging #68

@regisd
Copy link
Contributor

regisd commented Jul 10, 2015

A few other things that might be useful:

@artem-zinnatullin
Copy link
Owner

@ilya-murzinov, @regisd what do you think about StorIO https://github.com/pushtorefresh/storio? I suggest use StorIOSQLite or StorIOContentResolver with type mapping.

For the first refactor we can use it without RxJava, later we can switch to Rx too. As StorIO developer/maintainer I can help with any questions :)

@ilya-murzinov
Copy link
Collaborator Author

@artem-zinnatullin, yes, I think it's a good idea.
It should not be very hard to migrate to StorIO because we have only a few tables.

@artem-zinnatullin
Copy link
Owner

@ilya-murzinov great, I think it'll be better if you'll do it and give me feedback about APIs and documentation :)

@ilya-murzinov
Copy link
Collaborator Author

Yes, of course!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants