patternjavaMinor
Two Fragments in Landscape Mode Challenge
Viewed 0 times
challengefragmentsmodetwolandscape
Problem
I did the following code challenge for a job interview. They apparently didn't care for my solution and I'm not sure why.
What could I have done better?
Here is the challenge:
Create a small app with the following characteristics:
With the device in the portrait mode, display a fragment with a single
EditText instructing the user to enter his or her name. Below that,
display five buttons labelled "Button 1", "Button 2", "Button 3",
"Button 4" "Button 5". Tapping any of those buttons will swap in a new
fragment with a simple text field displaying the message, "Hello ! You
tapped Button 1" (or 2, and so on). Tapping the back button returns to
the original fragment with the five buttons.
When the device is in the landscape mode, display the two fragment
side-by-side within the same activity, but keep the behavior described
above (except that the back button will exit the app as there is no
fragment to go back to.)
[Feel free to use either the Fragment call introduced in API level 11
(android.app.Fragment) or the Fragment class found in the Android
support library (android.support.v4.app.Fragment).]
MainActivity.java:
```
public class MainActivity extends FragmentActivity {
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
//Remove title bar
this.requestWindowFeature(Window.FEATURE_NO_TITLE);
setContentView(us.jasonh.fragmentschallenge.R.layout.layout_main);
if (savedInstanceState != null) {
String name = savedInstanceState.getString("name");
EditText editText = (EditText) findViewById(us.jasonh.fragmentschallenge.R.id.editText);
editText.setText(name);
}
}
@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
EditText editText = (EditText) findViewById(us.jasonh.fragmentschallenge.R.id.editText);
What could I have done better?
Here is the challenge:
Create a small app with the following characteristics:
With the device in the portrait mode, display a fragment with a single
EditText instructing the user to enter his or her name. Below that,
display five buttons labelled "Button 1", "Button 2", "Button 3",
"Button 4" "Button 5". Tapping any of those buttons will swap in a new
fragment with a simple text field displaying the message, "Hello ! You
tapped Button 1" (or 2, and so on). Tapping the back button returns to
the original fragment with the five buttons.
When the device is in the landscape mode, display the two fragment
side-by-side within the same activity, but keep the behavior described
above (except that the back button will exit the app as there is no
fragment to go back to.)
[Feel free to use either the Fragment call introduced in API level 11
(android.app.Fragment) or the Fragment class found in the Android
support library (android.support.v4.app.Fragment).]
MainActivity.java:
```
public class MainActivity extends FragmentActivity {
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
//Remove title bar
this.requestWindowFeature(Window.FEATURE_NO_TITLE);
setContentView(us.jasonh.fragmentschallenge.R.layout.layout_main);
if (savedInstanceState != null) {
String name = savedInstanceState.getString("name");
EditText editText = (EditText) findViewById(us.jasonh.fragmentschallenge.R.id.editText);
editText.setText(name);
}
}
@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
EditText editText = (EditText) findViewById(us.jasonh.fragmentschallenge.R.id.editText);
Solution
import us.jasonh.fragmentschallenge.R
It's unusual and ugly that your package name
The common approach is to import the generated
So that you can simplify the source code and get rid of the ugly duplicated package names everywhere:
Search through your code and make sure to remove all occurrences of your package name (except
Simplify
In
You could just iterate from 1 to 5,
and in each cycle create a
configure it,
including the click listener,
and add it to the view.
No need for
Also, the click listener's implementation is not trivial,
embedding it inside the
which can be flagged as a warning by static analysis tools.
It would be better to use a named private inner class instead of an anonymous class for the click listener implementation.
Duplicated string constants
In
If you write
that's fragile,
because if you change it in one place,
you have to remember to change in the other too.
The common approach is to put this in a static constant variable:
And use this variable when saving and restoring "name".
The same goes for the
Misc
Instead of this:
I prefer this way:
For one thing, it's shorter. But maybe it's just a matter of taste.
It's unusual and ugly that your package name
us.jasonh.fragmentschallenge appears in many places in the source code:setContentView(us.jasonh.fragmentschallenge.R.layout.layout_main);The common approach is to import the generated
R file:import us.jasonh.fragmentschallenge.RSo that you can simplify the source code and get rid of the ugly duplicated package names everywhere:
setContentView(R.layout.layout_main);Search through your code and make sure to remove all occurrences of your package name (except
import statements).Simplify
In
onActivityCreated, you don't need to store an array of buttons.You could just iterate from 1 to 5,
and in each cycle create a
Button,configure it,
including the click listener,
and add it to the view.
No need for
Button[].Also, the click listener's implementation is not trivial,
embedding it inside the
onActivityCreated method makes the method's body a bit long,which can be flagged as a warning by static analysis tools.
It would be better to use a named private inner class instead of an anonymous class for the click listener implementation.
Duplicated string constants
In
onSaveInstanceState, and when you restore from it, you save and load "name".If you write
"name" in two places,that's fragile,
because if you change it in one place,
you have to remember to change in the other too.
The common approach is to put this in a static constant variable:
private static final String KEY_NAME = "name";And use this variable when saving and restoring "name".
The same goes for the
"message" key you pass between the fragments.Misc
Instead of this:
String.valueOf(editText.getText())I prefer this way:
editText.getText().toString()For one thing, it's shorter. But maybe it's just a matter of taste.
Code Snippets
setContentView(us.jasonh.fragmentschallenge.R.layout.layout_main);import us.jasonh.fragmentschallenge.RsetContentView(R.layout.layout_main);private static final String KEY_NAME = "name";String.valueOf(editText.getText())Context
StackExchange Code Review Q#64045, answer score: 7
Revisions (0)
No revisions yet.