Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I have two class:

InputForm.java

public class InputForm {
    private String brandCode;
    private String caution;

    public String getBrandCode() {
        return brandCode;
    }

    public void setBrandCode(String brandCode) {
        this.brandCode = brandCode;
    }

    public String getCaution() {
        return caution;
    }
    public void setCaution(String caution) {
        this.caution = caution;
    }
}

CopyForm.java

public class CopyForm {
    private boolean brandCodeChecked;
    private boolean cautionChecked;

    public boolean isBrandCodeChecked() {
        return brandCodeChecked;
    }

    public void setBrandCodeChecked(boolean brandCodeChecked) {
        this.brandCodeChecked = brandCodeChecked;
    }

    public boolean isCautionChecked() {
        return cautionChecked;
    }

    public void setCautionChecked(boolean cautionChecked) {
        this.cautionChecked = cautionChecked;
    }
}

I want to copy values from an InputForm to another if its corresponding property in CopyForm is true. This is what I do:

if(copyForm.isBrandCodeChecked()) {
    inputForm.setBrandCode(otherInputForm.getBrandCode());
}
if(copyForm.isCautionChecked()) {
    inputForm.setCaution(otherInputForm.getCaution());
}

The problem is I have many many properties. Writing many if statements seems ugly and bad programming practice. How to solve it? (I know reflection is not a good choice so I don't think about it)

share|improve this question
1  
Actually reflection came to mind when I read this. I am not very good at Java, so I haven't heard why using it is a bad idea. Why is this so? – Michael Zedeler Jun 11 at 7:29
@UwePlonus Nice tip! I've cleared all my erroneus comments. Thank for your help - It may be helpful to keep your last comment for others. – cl-r Jun 11 at 9:40
1  
@cl-r To remove a comment use the red cross beneath your signature. – Uwe Plonus Jun 11 at 9:52
add comment (requires an account with 50 reputation)

2 Answers

up vote 2 down vote

I think Apache commons BeanUtils might help you there. It uses reflection. Take this into consideration if performance is an issue...

We need 2 Steps:

1st create a method that copies a property if a condition is met

2nd create a copy method that passes all fields to the first method

Here we go...

public class InputForm {
    [...]

    private void copyProperty(InputForm srcForm, String field, boolean condition) {
        if(condition) {
            PropertyUtils.setProperty(this, field, PropertyUtils.getProperty(srcForm, field));
        }
    }

    public void copyFromOtherForm(InputForm srcForm, CopyForm conditions) {
        copyProperty(srcForm, "brandCode", conditions.isBrandCodeChecked());
        copyProperty(srcForm, "caution", conditions.isCautionChecked());
        [...]
    }
}

if you're still not pleased with that you could use reflection to invoke "is"+field+"Checked" on your CopyForm instance and change the calls to:

copyProperty(srcForm, conditions, "field");
share|improve this answer
Why using an external package when the language offers you what you need already (see my post)? – fge Jun 11 at 13:55
add comment (requires an account with 50 reputation)
up vote 0 down vote

You may want a static method in your InputForm class, named, for instance, .fromCopyForm() taking a CopyForm as an argument and returning an InputForm:

public static InputForm fromCopyForm(final CopyForm copyForm)
{
    final InputForm ret = new InputForm();
    // all tests here
    return ret;
}

Then in your code:

final InputForm form = InputForm.fromCopyForm(copyForm);
share|improve this answer
where is the magic that fills the fields of the new InputForm depending on the flags in CopyForm? – Marco Forberg Jun 11 at 13:58
Nowhere, why need magic? Just simple checks, written once, that's it; and no reflection needed – fge Jun 11 at 13:59
"simple checks" = many if statements = the thing the op wants to get rid of? – Marco Forberg Jun 11 at 14:02
No, simple checks in the static factory method; none where the code is actually used. – fge Jun 11 at 14:03
1  
i suggest you read the question again... – Marco Forberg Jun 11 at 14:09
show 5 more commentsadd comment (requires an account with 50 reputation)

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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