Simplify complex HomeConfig class and make the class easier to read and maintain




Firefox for Android
3 years ago
2 years ago


(Reporter: sebastian, Unassigned, Mentored)


Firefox Tracking Flags

(Not tracked)


(Whiteboard: [lang=java])

Follow-up from bug 1172136.

Quoting from mhaigh bug 1172136 comment#12:
> Good overall but I have a critisism about the existing architecture of
> which stems from my dislike of using null objects.  I'd prefer 
> if all config objects implemented a common interface and either there was also 
> a EmptyConfig object which we could instantiate if the config in question 
> wasn't used, or the config objects could handle being empty.  Together with 
> this I'd prefer if objects were responsible for themselves, so instead of this 
> codeblock:
>       if (mHeaderConfig != null) {
>           json.put(JSON_KEY_HEADER, mHeaderConfig.toJSON());
>       }
> instead we passed the json object in to the mHeaderConfig object and it assign 
> whatever values are needed to the json object.

HomeConfig has been growing to over 1600 lines now. This bug is about reducing the technical dept that has been accumulated by adding features to HomeConfig.

Some ideas to make the class simpler and easier to maintain:

* Move some of the 14 nested classes outside

* Reduce number of null checks by using the "null object pattern" (see quote above)

* A bunch of nested *Config classes are very similar. Maybe they can share a base class or implement a shared interface in order to generalize code using them.
Whiteboard: [lang=java]

Comment 1

2 years ago
Hi, I'd gladly work on this issue, if it is still relevant :)
(In reply to Anita from comment #1)
> Hi, I'd gladly work on this issue, if it is still relevant :)

Hi Anita! :)

We are currently experimenting with different panels, and thinking about simplifying things by combining some of the panels. With that work happening currently I think we probably should not start to refactor those home panel classes at the same time. In fact I'd refrain from refactoring them at all without the need to change something in this area.

However if home panels are an area that you would like to work on: Bug 1192788 looks like a good start. Or if the area does not matter much this is a good list of mentor bugs to start with:
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.