javalite / javalite

JavaLite is a cohesive collection of frameworks designed from ground up to add pleasure back to your daily life
http://javalite.io
Other
855 stars 213 forks source link

Review methods names improving fluent interface #321

Open ericbn opened 9 years ago

ericbn commented 9 years ago

Migrate some remaining getX() methods throughout the code (keeping them deprecated) to the x() form. They read much more more like a "lite" Java, ruby-inspired, and read better as fluent interface.

ericbn commented 9 years ago

Guava and Joda Time may provide good inspiration for this!

ericbn commented 9 years ago

We should also avoid prefixing accessors with get or is, or prefixing mutators with set.

Accessor and mutator methods for boolean values should read as verbs in passive voice (or adjectives). Examples:

public boolean frozen();
public boolean modified();
public boolean saved();
public boolean valid();
public boolean paused();
public void paused(boolean paused); // mutator

Accessor and mutator methods for non-boolean values should read as substantives/nouns. Examples:

protected Map<String, Object> attributes();
public String query();
public long time();
public String message();
public void message(String message); // mutator
ipolevoy commented 9 years ago

@ericbn I would suggest that we add these (need to review each one), but also provide standard methods, such as isFrozen, with "get" and "set". Consider two versions of the same synonyms. My logic is as follows. I personally prefer simple ones as you know. However, this is strange for most Java developers. Another positive that comes from having getters and setters is autocomplete. IDEs offer a list of all methods that are mutators or accessors if you type model.get an type Ctrl+Space.

ipolevoy commented 9 years ago

there are quite a few synonyms like these JavaLite, but there was never a systematic approach to them.

ericbn commented 9 years ago

@ipolevoy, I don't think we should have two versions of each method, for that would mean LOTS of duplicate code. I think that would go against the idea of a light and simple framework.

Even a simple class will be affected forever:

public class QueryExecutionEvent {
    private final String query;
    private final long time;

    public QueryExecutionEvent(String query, long time) {
        this.query = query;
        this.time = time;
    }

    public String getQuery() {
        return query;
    }
    public String query() {
        return query;
    }
    public long getTime() {
        return time;
    }
    public long time() {
        return time;
    }
}

But I agree completely -- and that was the idea behind this issue -- that we need a systematic approach. We have a if (Util.blank(str)) on one hand and if (model.isFrozen()) on the other. It would be good to stick to one approach and use it everywhere -- and I mean 100% of the code by that.

I think it is up to you to decide which one must be followed.

ipolevoy commented 9 years ago

@ericbn , you do not need to replicate code:


    public String getQuery() {
        return query;
    }
    public String query() {
        return getQuery();
    }

We do have some of this code in the framework already. I'm split which way it needs to go, as both approaches have pros and cons.

ipolevoy commented 9 years ago

Also, removing methods is something that you want to do, but that will force people using the framework to make code changes. For many, that will be an annoyance and unnecessary superficial changes. When Play rolled a new release v 2.0, many abandoned using it for that same exact reason.

ericbn commented 9 years ago

@ipolevoy, replacing an API by a better is one is of the ways products can get continuously better over time... Most major version changes imply a different API.

ipolevoy commented 9 years ago

@ericbn , this is more like spinning the wheels, since the best way to make the project better is to place it into hands of more developers. This is where we need to channel the energy :)