2
\$\begingroup\$

I have the following class:

public final class ViewUtils {

    public static void setSize(View view, int width, int height) {
        ViewGroup.LayoutParams layoutParams = (ViewGroup.LayoutParams) view.getLayoutParams();
        layoutParams.width = width;
        layoutParams.height = height;
        view.requestLayout();
    }

    public static void setWidth(View view, int width) {
        setSize(view, width, view.getHeight());
    }

    public static void setHeight(View view, int height) {
        setSize(view, view.getWidth(), height);
    }

    public static boolean isRightEdgeOfScrollViewHasBeenReached(HorizontalScrollView scrollView) {
        View childView = scrollView.getChildAt(scrollView.getChildCount() - 1);
        return childView.getRight() == (scrollView.getWidth() + scrollView.getScrollX());
    }

    // Other methods will be added later

}

But I don't like the name isRightEdgeOfScrollViewHasBeenReached. I think it would be better if the method name didn't contain word ScrollView but is placed in a class which would be specialized only on the ScrollView.

Should I place this method in an inner class ViewUtils.ScrollView or in a separate class ScrollViewUtils?

I think the first option would be better, because creating a separate class for a few methods (currently only one method) is too "expensive".

And help me choose more appropriate name for the method isRightEdgeOfScrollViewHasBeenReached.


Summarizing the question

The choice is between two options:

To have all methods inside a single class:

public class ViewUtils {

    public static void methodForAllViews1() { }

    public static void methodForAllViews2() { }

    public static void methodForAllViews3() { }


    public static class TextView {

        public static void methodForTextViews1() { }

        public static void methodForTextViews2() { }
    }

    // etc.

}

To separate all methods and to place all util classes in the same package:

- utils
    - view
        * ViewUtils.java
        * TextViewUtils.java
        * ScrollViewUtils.java
\$\endgroup\$
3
\$\begingroup\$

Refactoring and renaming isRightEdgeOfScrollViewHasBeenReached

How about like this:

public static boolean isAtRightEdge(ViewGroup viewGroup) {
    View childView = viewGroup.getChildAt(viewGroup.getChildCount() - 1);
    return childView.getRight() == viewGroup.getWidth() + viewGroup.getScrollX();
}

I haven't tested this, but it compiles. As such, I don't really see a reason to limit the method to take a HorizontalScrollView as parameter. Do let me know if you see some shortcomings in your tests.

Also, I removed the unnecessary parentheses from the return statement.

Organizing helper methods and utility classes

Here are some general tips:

  • Only move a method from your real class to a utility class if you will actually use it in at least two unrelated classes, right now. Don't add utility classes that "seem like a good idea", or if you're sure "you'll definitely use it sometime really soon". Don't speculate, wait until you have an actual use case.

  • Don't nest utility classes. Nested classes were not intended for this purpose, it would be misusing the language. This page in the docs explains when it is appropriate to use nested classes.

  • If you start to have too many methods in the utility class, or when it makes sense, split to multiple classes. (As you proposed in option 2.)

  • Avoid overloaded helper methods with the same number of parameters. (This is just a tip for the future, there was no such example in your post.)

  • Prevent instantiation of utility classes by adding a private parameterless constructor, for example:

    private ViewUtils() {
        // utility class, should not instantiate
    }
    
\$\endgroup\$
2
\$\begingroup\$

Here is third option:

3) Stop creating so many utils and write your code in an OOP way.

Utility classes or methods are, in most circumstances, a very bad anti-pattern.

Why don't you have a View class that can make use of ScrollView or TextView using composition and on those classes you can just call methods such as void SetDimensions(Dimensions newDimensions) or bool IsWhateverCondition() and so on.

For further proofing your design we would need at least an UML diagram of your classes with semantic explanations. There are too little details to be able to make a more in-depth analysis.

Regarding your isRightEdgeOfScrollViewHasBeenReached, although I don't know exactly what are the semantics of this methods (you should had included this in your question) I would guess it would suit something as double ScrollProgress() that returns the progress, in percents, of your scroll bar (I think that is a scroll bar). You can have an additional method then to check what you want, maybe called IsScrolledToEnd or such. Right, Edge and HasBeenReached don't add too much value to your method name. Those should be deducible from the context. The method can be generic enough and shouldn't have any concept of View or Right.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.