Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Fix bug where onDetach is called before onAttach #66

Merged
merged 1 commit into from
May 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions scoop/src/main/java/com/lyft/scoop/ViewController.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@

public abstract class ViewController {

private boolean isDetaching = false;
private boolean attached;
private Scoop scoop;
private View view;

final void attach(View view) {
this.attached = true;
this.view = view;
onAttach();
this.attached = true;
if (this.isDetaching) {
detach(view);
}
}

public void onAttach() {}
Expand All @@ -21,9 +25,13 @@ protected final boolean attached() {
}

final void detach(View view) {
onDetach();
this.view = null;
this.attached = false;
this.isDetaching = true;
if(this.attached) {
onDetach();
this.view = null;
this.attached = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itspbj Do we use attached anywhere in the code right now? may be delete it.

this.isDetaching = false;
}
}

public void onDetach() {}
Expand Down
69 changes: 69 additions & 0 deletions scoop/src/test/java/com/lyft/scoop/ViewControllerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.lyft.scoop;

import android.content.Context;
import android.view.View;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;

import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;

@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class ViewControllerTest {
private static final String TEST_RESULT = "TEST_RESULT";

private MockViewController viewController;
private View mockView;

@Before
public void setUp() {
viewController = new MockViewController();
mockView = new TestView(RuntimeEnvironment.application);
}
@Test
public void onAttachFirst() {
viewController.attach(mockView);
assertTrue(viewController.attached());
viewController.detach(mockView);
assertFalse(viewController.attached());
}

@Test
public void onDetachFirst() {
viewController.detach(mockView);
assertFalse(viewController.attached());
viewController.attach(mockView);
assertFalse(viewController.attached());
}

private class MockViewController extends ViewController {
private String variable = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itspbj call this MockViewController. Use public fields instead private


@Override
public void onAttach() {
variable = TEST_RESULT;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itspbj what this varialbe is used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lexer Simulate a variable being used in onDetach that was initialized in onAttach. In our code this throws an NPE in some cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itspbj ok

}

@Override
public void onDetach() {
variable.toString();
}

@Override
protected int layoutId() {
return 0;
}
}

static class TestView extends View {

public TestView(Context context) {
super(context);
}
}
}