PS sms phone call history

PS sms phone call history

[mail2coolgarg, 2014-04-25 05:07:21]

fix comments. and send again for review.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
File spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java
(right):

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java:208:
v.getContext(), sro.storeId);
instead of v.getContext(), you can use activity here.
no need to pass v then.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java:211:
.findViewById(R.id.lvQuestionAnswer);
use only one listview for both qna and sms/phone  list. you just need to change
adapter and it will work fine.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
File spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java (right):

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:18: Context
mContext;
no need to have context here.

activity is super class of context, so you can use activity, wherever you are
using context.

make all members as private.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:33: private
Activity activity;
you are not using activity anywhere in viewholder. then remove it.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:34: private View
convertView;
you dont need to declare convertView here.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:39: public
ViewHolder(View v, Activity activity,
v => converView

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:40:
StoreSmsPhoneCallObjectModel sspco) {
sspco,
why are you passing when you are not using it at all?? huh!!

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:54: this.sspco =
sspco;
What is the use of having a member variable sspco. while you can do all things
with sspco, which is passed in method. so remove member variable.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:60: public void
setImage(StoreSmsPhoneCallObjectModel sspco) {
Why do you have setImage and resetImage, while these are not doing anything

remove them.

https://codereview.appspot.com/91790045/diff/1/spfaq/src/app/shoppoke/faq/sto...
spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:69: public View
getView(int position, View v, ViewGroup parent) {
please dont remove @Override tag with any method which is overrided.

[mail2coolgarg, 2014-04-25 05:09:02]

On Fri, Apr 25, 2014 at 10:37 AM, <mail2coolgarg@gmail.com> wrote:

> fix comments. and send again for review.
>
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreContactInformationHandler.java
> File
> spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java
> (right):
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreContactInformationHandler
> .java#newcode208
> spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler
> .java:208:
> v.getContext(), sro.storeId);
> instead of v.getContext(), you can use activity here.
> no need to pass v then.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreContactInformationHandler
> .java#newcode211
> spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler
> .java:211:
> .findViewById(R.id.lvQuestionAnswer);
> use only one listview for both qna and sms/phone  list. you just need to
> change adapter and it will work fine.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java
> File spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java
> (right):
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode18
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:18: Context
> mContext;
> no need to have context here.
>
> activity is super class of context, so you can use activity, wherever
> you are using context.
>
> make all members as private.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode33
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:33: private
> Activity activity;
> you are not using activity anywhere in viewholder. then remove it.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode34
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:34: private
> View convertView;
> you dont need to declare convertView here.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode39
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:39: public
> ViewHolder(View v, Activity activity,
> v => converView
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode40
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:40:
> StoreSmsPhoneCallObjectModel sspco) {
> sspco,
> why are you passing when you are not using it at all?? huh!!
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode54
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:54:
> this.sspco = sspco;
> What is the use of having a member variable sspco. while you can do all
> things with sspco, which is passed in method. so remove member variable.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode60
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:60: public
> void setImage(StoreSmsPhoneCallObjectModel sspco) {
> Why do you have setImage and resetImage, while these are not doing
> anything
>
> remove them.
>
> https://codereview.appspot.com/91790045/diff/1/spfaq/src/
> app/shoppoke/faq/storedetail/StoreWallAdapter.java#newcode69
> spfaq/src/app/shoppoke/faq/storedetail/StoreWallAdapter.java:69: public
> View getView(int position, View v, ViewGroup parent) {
> please dont remove @Override tag with any method which is overrided.
>
> https://codereview.appspot.com/91790045/
>



-- 


----
Himanshu Garg
CEO - ShopPoke

[mail2coolgarg, 2014-04-25 06:12:40]

more comments, please fix them and send again for review.

if confusion, call me.

https://codereview.appspot.com/91790045/diff/20001/spfaq/src/app/shoppoke/faq...
File spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java
(right):

https://codereview.appspot.com/91790045/diff/20001/spfaq/src/app/shoppoke/faq...
spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java:204:
private void populateWall(View v) {
no need to pass View here.

https://codereview.appspot.com/91790045/diff/20001/spfaq/src/app/shoppoke/faq...
spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java:209:
ListView list;
dont declare and then assign, do it together.

Use same listview, remove extra listview from xml file itself

https://codereview.appspot.com/91790045/diff/20001/spfaq/src/app/shoppoke/faq...
spfaq/src/app/shoppoke/faq/storedetail/StoreContactInformationHandler.java:212:
list.setVisibility(v.GONE);
why are you making it invisible and the visible.