Should I do things like this?

I'm only started learning guava. So I don't know best practices and etc. These is code (it binding some classes and put in Order by input collection):

public ImmutableList<ModelBean> toBean(Collection<Shape> model) {
    return ImmutableList.copyOf(Collections2.transform(Ordering.f开发者_StackOverflowrom(new Comparator<Shape>() {
        @Override
        public int compare(Shape o1, Shape o2) {
            return 0; //here placed some one line logic
        }
    }).sortedCopy(model), new Function<Shape, ModelBean>() {
        final ModelBeanCreator binder = new ModelBeanCreator();

        @Override
        public ModelBean apply(Shape input) {
            return binder.createModelBean(input);
        }
    }));
}

So, should I separate it in several operations?

upd what does it do? It takes collection. Sorts it. Maps each object to other one. Creates new ImmutableList and return it.

---

I think it's generally okay to combine multiple operations in one call (it's just a shame Java doesn't have extension methods to make it prettier), but I'd advise you not to include the logic inline. With anonymous classes, it just gets messy.

Instead, declare your predicates, orderings, projections etc as constants:

private static Function<Shape, ModelBean> MODEL_BEAN_PROJECTION =
    new Function<Shape, ModelBean>() {
    final ModelBeanCreator binder = new ModelBeanCreator();

    @Override
    public ModelBean apply(Shape input) {
        return binder.createModelBean(input);
    }
};

then you can use MODEL_BEAN_PROJECTION in your method call later on. That way you can get code which is actually reasonably easy to read, despite doing a lot.

On the other hand, the option of using a local variable to describe what you've got so far at each stage of the transformation is also potentially useful. It's often worth trying some code both ways, and seeing which you find more readable - and ask a colleague, too. Also experiment with different whitespace options - I find that the readability difference between code using whitespace sensibly and not can be massive.

---

As Jon Skeet pointed out, moving the anonymous objects into constants or separate declarations can make the code more readable. If you use the same anonymous objects elsewhere, they can be scoped at the class level but you could also scope them as final declarations in the method if you really wanted to prevent their reuse.

public ImmutableList<ModelBean> toBean(Collection<Shape> model) {
    // I pulled out the first anonymous class object into a method scoped
    // reference and gave it some meaningful name based on what it does.
    final Comparator<Shape> shapeComparator = new Comparator<Shape>() {
        @Override
        public int compare(Shape o1, Shape o2) {
            return 0; //here placed some one line logic
        };

    // Also pulled out the second anonymous object into a method scoped
    // reference with meaningful name.
    final Function<Shape, ModelBean> sortFunc = new Function<Shape, ModelBean>() {
        final ModelBeanCreator binder = new ModelBeanCreator();

        @Override
        public ModelBean apply(Shape input) {
            return binder.createModelBean(input);
        }
    }; 

    // Now the transformation is much simpler without much work.
    return ImmutableList.copyOf(Collections2.transform(Ordering.from( shapeComparator ).sortedCopy(model), sortFunc ));
}

---

Another question is whether to use functional programming constructs, such as Collections2.transform(), at all. Instead, you could say

ImmutableList.Builder<ModelBean> builder = ImmutableList.builder();
for (Shape shape : sortedShapes) {
  builder.add(binder.createModelBean(shape));
}
return builder.build();

I prefer explicit iteration when you want to make a separate copy, since it's easier to read and write such code.