HiveBrain v1.2.0
Get Started
← Back to all entries
gotchajavaMinor

Why does the new ADT create a static inner class Fragment by default?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
whythenewcreateadtdefaultdoesfragmentinnerclass

Problem

Honestly, I can't help but feel that this is done merely to confuse newcomers. Most of the errors on Stack Overflow by complete Android newbies mostly stem from that they have a static inner class Fragment that they don't understand how it works, why it's there, and try to use Activities even though they don't fully understand the concept behind what is happening.

I must admit, I had trouble understanding the PlaceholderFragment approach too, and using static inner classes isn't really extensible at all. The first thing you'd have to do is create an actual class outside - but why do newbies have to do that?

I think this could be much more efficient if they used a project structure similar to the following simple fragment-based android project structure:



  • src




  • wholepackagename




  • activity




  • MainActivity




  • fragment




  • FirstFragment



  • SecondFragment






  • res




  • layout



  • values



  • ...





With the code of

src/wholepackagename/activity/MainActivity:

`public class MainActivity extends FragmentActivity implements FirstFragment.Callback
{
@Override
protected void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);

getSupportFragmentManager().addOnBackStackChangedListener(new OnBackStackChangedListener()
{
public void onBackStackChanged()
{
int backCount = getSupportFragmentManager().getBackStackEntryCount();
if (backCount == 0)
{
finish();
}
}
});

if (savedInstanceState == null)
{
getSupportFragmentManager().beginTransaction().add(R.id.main_container, new FirstFragment()).addToBackStack(null).commit();
}
}

@Override
public void firstFragmentCallback()
{
getSupportFragmentManager().beginTransaction().replace(R.id.ma

Solution

I totally agree that it is better to have a Fragment in it's own class rather than as a static inner class. I don't think I've seen any cases where it's been a static inner class, but perhaps I'm just lucky (or have a bad memory).

Overall your code is very clean. And I really mean very clean. Probably the cleanest code I've read today (that includes my own code).

If this is meant to be a tutorial kind of example code though, there's a few things I'd like to say.

First of all, your braces are using the C# convention. The Java convention is to use:

xxxx {
    // stuff
}


That is, the opening brace { goes on the same line and not on its own.

As you're coding Java, I'd recommend using the Java convention. At least your style is consistent though, but as it seems to be code for teaching others, it's important to teach the correct conventions.

public static interface Callback
{
    void firstFragmentCallback();
}


Excellent little interface there, I'm just afraid that with this naming you'll end up with ten interfaces named Callback. Naming it FirstFragmentCallback would be better.

These constructors can be removed completely without altering the behavior of your application in any way. As all they do is call super(), which is implicitly called anyway, and that you don't have any other constructors, they're not required at all.

public FirstFragment()
{
    super();
}

public SecondFragment()
{
    super();
}


Log.e(getClass().getSimpleName(), activity.getClass().getSimpleName() + " must implement Callback interface!", e);


It's excellent that you're logging this and re-throwing the exception, I would however log the canonical name (some.package.SomeActivity) and not just the simple name (SomeActivity).

An alternative here though would be to provide a setCallback method. I am not sure what the best practice is actually with regards to this, but well... it's an alternative. It would add some flexibility as the activity itself doesn't have to be the one implementing the callback.

FirstFragment fragment = new FirstFragment();
fragment.setCallback(new FirstFragment.Callback(){ ... });
getSupportFragmentManager().beginTransaction().add(R.id.main_container, fragment).addToBackStack(null).commit();


Toast.makeText(getActivity(), "This is an example!", Toast.LENGTH_LONG).show();


OK, OK, I know it's an example, but still... you have a res/values/strings.xml already with some additional strings, it would be preferred to use a string resource here as well.

Again, overall it's very neat and clean. These were all the issues I've found. Good job.

Code Snippets

xxxx {
    // stuff
}
public static interface Callback
{
    void firstFragmentCallback();
}
public FirstFragment()
{
    super();
}


public SecondFragment()
{
    super();
}
Log.e(getClass().getSimpleName(), activity.getClass().getSimpleName() + " must implement Callback interface!", e);
FirstFragment fragment = new FirstFragment();
fragment.setCallback(new FirstFragment.Callback(){ ... });
getSupportFragmentManager().beginTransaction().add(R.id.main_container, fragment).addToBackStack(null).commit();

Context

StackExchange Code Review Q#57543, answer score: 7

Revisions (0)

No revisions yet.