What is wrong with this code #02

Given that the functionality of the method toGenericObject must be preserved; there is something obviously wrong in this code, can you find it?

If so, can you think of an easy solution?


public MyObject {

	private String myField;
	 
	... (imagine multiple fields) ...

	public GenericObject toGenericObject() {
		GenericObjectFactory genericObjectFactory = GenericObjectFactory.getInstance();
		GenericObject genericObject = genericObjectFactory.create("myObject");
		genericObject.setField("myField", myField);
		// imagine multiple lines using genericObject.setField("field", field)
		return genericObject;
	}

}

Comments

    1. Do you really think i approve this code? 😉 and since when has a language has anything to do with dependency management? 🙂

      I would love to do ruby some time.

      1. Btw, if you thought ” too verbose” was the answer of thus post, then please look again. 🙂

  1. Doing this setField(‘myField’, myField) for all class variables results in that genericObject’s class variables refer to the same objects as myObject, myField.clone() should be user to construct the New genericObject.

    1. On the other hand, if you do not want to clone the myObject to a generic object, but rather treat the myObject as a generic object, first thing that comes to mind is that GenericObject should be the parentclass of MyObject.

      1. Both good points Bart. Funny how many things can be found in such a small piece of code.

  2. The whole idea of creating an object with “generic” fields is bothering me but the first thing I would do is extract the following code in a seperate methode so you can mock it when testing:

    GenericObjectFactory genericObjectFactory = GenericObjectFactory.getInstance();

    to something like this:

    public GenericObjectFactory getGenericObjectFactory() {
    return GenericObjectFactory.getInstance();
    }

    The other thing I would do is to make the following code explicit:

    GenericObject genericObject = genericObjectFactory.create(“myObject”);
    genericObject.setField(“myField”, myField);

    It would like something like this for the readability:

    GenericObject genericObject – genericObjectFactory.createMyObjectWithMyField(myField);

    1. The getInstance of the GenericObjectFactory is a pain yes. Depending on how invasive you want/can be in the method signature there would be more options then extracting to a getGenericObjectFactory method.

      Also, there are more fields on this object. So making explicit is a good way to go I think, but “withMyField” would not suffice as there are other fields as well.

      Nice input Arjan 🙂

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.