Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [ice-dev] interface design

Robert and I reviewed these in his code review this afternoon. He has added tickets to look into each of these.

Jay

On Fri, Feb 26, 2016 at 2:59 PM, Greg Watson <greg@watson.earth> wrote:
I noticed that there are some places in the eavp code where you’re returning an IController the casting it into an IWireFramePart. I think it would be good practice if IWireFramePart at least extends IController if you’re going to do this.

I also see some code like this, where you have a lot of casts:

        private void recursiveSetWireframe(IController target, boolean wireframe) {

                // Set this object to the correct mode
                ((IWireFramePart) target).setWireFrameMode(wireframe);

                // Iterate over each of its children, setting them to the correct mode
                for (IController child : target
                                .getEntitiesByCategory(MeshCategory.CHILDREN)) {
                        ((IWireFramePart) child).setWireFrameMode(wireframe);
                }
        }


I think you could avoid a lot of this if the target argument to recursiveSetWireframe was type IWireFramePart, and since FXGeometryCanvas assumes that the controller is IWireFramePart, that should be the type used in the constructor.

You could also specify the type in getEntitiesByCategory(), e.g.

        <T extends IController> ArrayList <T> getEntitiesByCategory(IMeshCategory category, Class<T> type)

then you could say:

        for (IWireFramePart child : target.getEntitiesByCategory(MeshCategory.CHILDREN, IWireFramePart.class)) {
                …
        }

This would also help in other situations to avoid casts:

        List<PipeController> primaryPipe = input
                                        .getEntitiesByCategory(ReactorMeshCategory.PRIMARY_PIPE, PipeController.class);

I’m also a bit confused about the getRepresentation() method on IController. This returns an Object, but everywhere it is used it is cast into a Group. Is there some expectation that it may be something else other than a Group? Is it necessary to be a completely arbitrary object, or could there be an interface that is returned instead?

Greg
_______________________________________________
ice-dev mailing list
ice-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/ice-dev



--
Jay Jay Billings
Oak Ridge National Laboratory
Twitter Handle: @jayjaybillings

Back to the top