Refactoring help…property-based object or lots of member fields?

I'm currently refactoring a class which currently looks something like this:

 class SomeModel {
    private String displayName;
    private int id;
    private boolean editable;
    private int minimumAllowedValue;
    private int maximumAllowedValue;
    private int value;
    // etc. etc.... a bunch (10+) of other fields...

    // and then tons of setters and getters for these fields, some of the
    // setters have restrictions depending on other settings, like you can't
    // set the maximum lower than the minimum, etc.
    // ...
 }

My question is: is this really the best way to go or should I refactor all these fields into more of a property-based structure (with simply two methods setProperty and getProperty)?

Another possible refactoring would be to extract "properties" belonging together into classes of their own, such as the max/min structure into an "AllowedRange" object or something.

Ideas?

Answers


Don't do it. It's fine.

In the future just use an IDE like Eclipse to autogenerate Javabean classes, properties and/or getters&setters in a few clicks so that you don't need to type all those code down again and again.


  • You can take a look at BeanProperties - it supports constraints, events, etc.
  • Also check Project Lombok
  • You can have a Map and a bunch of constants as keys and use the setProperty to impose some of the limits. Or even to register validators.
  • If this object is supposed to be manipulated by other tools/frameworks (spring, hibernate, etc), then of course this refactoring is not an option - it should conform to the JavaBeans spec
  • ultimately, why do you need to refactor it in the first place? Isn't it doing its job well?

Of the fields you listed, I would - as you suggest - refactor minimumAllowedValue and maximumAllowedValue into a Range class, then replace them with, say, allowedRange. If I started to see that value and its allowedRange were a common pattern, I might move on to, say, a BoundedValue class. It's impossible, without seeing all the other fields, to say what to do with them, but too many fields is definitely a smell. The solution depends on their inter-relatedness, but the min & max --> range and value & range --> boundedValue examples give you some idea of the kinds of techniques to apply.

Also, keep the Single Responsibility Principle in mind.


Need Your Help

scrollTop duplicating values each time function is called

javascript jquery

I have 2 divs that each time one of them is clicked it scrolls down to a common content div called .nbm_specs. The first time you click it scrolls correctly, however any subsequent clicks make the

About UNIX Resources Network

Original, collect and organize Developers related documents, information and materials, contains jQuery, Html, CSS, MySQL, .NET, ASP.NET, SQL, objective-c, iPhone, Ruby on Rails, C, SQL Server, Ruby, Arrays, Regex, ASP.NET MVC, WPF, XML, Ajax, DataBase, and so on.