Collectible Card Game Headquarters

Maybe we should remove a requirement to add javadocs to each and every method?

This sample of code

Code: Select all

/**
 * <p>Quest_Assignment class.</p>
 *
 * @author Forge
 * @version $Id: Quest_Assignment.java 10192 2011-09-03 09:22:29Z slapshot5 $
 */
public class Quest_Assignment {
    private int id;
    private int requiredNumberWins;
    private int computerLife;

    private long creditsReward;

    private String name;
    private String desc;
    private String difficulty;
    private String cardReward;
    private String iconName;

    private boolean repeatable;

    private ArrayList<CardPrinted> cardRewardList = new ArrayList<CardPrinted>();

    private CardList human = new CardList();
    private ArrayList<String> compy = new ArrayList<String>();

    /**
     * <p>Setter for the field <code>id</code>.</p>
     *
     * @param idIn a int.
     */
    public final void setId(final int idIn) {
        this.id = idIn;
    }

    /**
     * <p>Getter for the field <code>id</code>.</p>
     *
     * @return a int.
     */
    public final int getId() {
        return id;
    }

    /**
     * <p>Setter for the field <code>creditsReward</code>.</p>
     *
     * @param creditsRewardIn a long.
     */
    public final void setCreditsReward(final long creditsRewardIn) {
        this.creditsReward = creditsRewardIn;
    }

    /**
     * <p>Getter for the field <code>creditsReward</code>.</p>
     *
     * @return a long.
     */
    public final long getCreditsReward() {
        return creditsReward;
    }

    /**
     * <p>Setter for the field <code>name</code>.</p>
     *
     * @param nameIn a {@link java.lang.String} object.
     */
    public final void setName(final String nameIn) {
        this.name = nameIn;
    }

    /**
     * <p>Getter for the field <code>name</code>.</p>
     *
     * @return a {@link java.lang.String} object.
     */
    public final String getName() {
        return name;
    }

Really looks like this.

Code: Select all

/**
 * SPAM SPAM SPAM SPAM SPAM
 *
 * SPAM SPAM SPAM
 * SPAM SPAM SPAM SPAM SPAM SPAM SPAM 10192 2011-09-03 09:22:29Z slapshot5 $
 */
public class Quest_Assignment {
    private int id;
    private int requiredNumberWins;
    private int computerLife;

    private long creditsReward;

    private String name;
    private String desc;
    private String difficulty;
    private String cardReward;
    private String iconName;

    private boolean repeatable;

    private ArrayList<CardPrinted> cardRewardList = new ArrayList<CardPrinted>();

    private CardList human = new CardList();
    private ArrayList<String> compy = new ArrayList<String>();

    /**
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     *
     * SPAM SPAM SPAM
     */
    public final void setId(final int idIn) {
        this.id = idIn;
    }

    /**
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     *
     * SPAM SPAM SPAM
     */
    public final int getId() {
        return id;
    }

    /**
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     *
     * SPAM SPAM SPAM SPAM SPAM SPAM
     */
    public final void setCreditsReward(final long creditsRewardIn) {
        this.creditsReward = creditsRewardIn;
    }

    /**
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     *
     * SPAM SPAM SPAM
     */
    public final long getCreditsReward() {
        return creditsReward;
    }

    /**
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     *
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     */
    public final void setName(final String nameIn) {
        this.name = nameIn;
    }

    /**
     * SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM SPAM
     *
     * SPAM SPAM SPAM
     */
    public final String getName() {
        return name;
    }

Would not this code be better in the following shape?

Code: Select all

// $Id: Quest_Assignment.java 10192 2011-09-03 09:22:29Z slapshot5 $
public class Quest_Assignment {
    private int id;
    private int requiredNumberWins;
    private int computerLife;

    private long creditsReward;

    private String name;
    private String desc;
    private String difficulty;
    private String cardReward;
    private String iconName;

    private boolean repeatable;

    private ArrayList<CardPrinted> cardRewardList = new ArrayList<CardPrinted>();

    private CardList human = new CardList();
    private ArrayList<String> compy = new ArrayList<String>();

    public final void setId(final int idIn) {
        this.id = idIn;
    }

    public final int getId() {
        return id;
    }

    public final void setCreditsReward(final long creditsRewardIn) {
        this.creditsReward = creditsRewardIn;
    }

    public final long getCreditsReward() {
        return creditsReward;
    }

    public final void setName(final String nameIn) {
        this.name = nameIn;
    }

    public final String getName() {
        return name;
    }

That would mean no waste of space and attention for unsignifincat things.
May we agree that trivial methods don't require any javadocs?

PS: some fields should be readonly, that means even no setter was needed in that case, not just the javadoc.
And in case a field is atomic and readonly, we might just declare it public final and bother not with those gayers/sayers and docs for them

Javadocs are great for code that is used externally... or utility functions. But not so useful for every method of every class, that gets used once or twice.

I would love to... remove them somehow
The ones which mean no sense, just consume space... and... maybe we should remove that reqiuirement from checkstyle.

Note that if you remove the requirement for javadocs on some public methods you remove it for all public methods. Checkstyle does not determine how "trivial" a method is. Also, in your example you cite that the class should not have Javadoc as well. That is a very bad practice. All classes should have documentation explaining what they do. Just because there are examples of poor Javadocs does not mean that Javadocs should be tossed out.

jendave wrote:Note that if you remove the requirement for javadocs on some public methods you remove it for all public methods. Checkstyle does not determine how "trivial" a method is. Also, in your example you cite that the class should not have Javadoc as well. That is a very bad practice. All classes should have documentation explaining what they do. Just because there are examples of poor Javadocs does not mean that Javadocs should be tossed out.

Is there a problem about it? That looks really good.
So only non-trivial methods that really need docs will receive some.

The mentioned class does not need any javadocs at all, because it contains just a number of fields with their getters/setters. So a class with 13 fields turns into a 275-line mess. Would you agree with me, that understanding of this class will become easier if we just drop all javadocs from it? (because you may just scroll through the code and get the idea) The only javadoc worth keeping there is the one that would describe class's designation, that is when and for which purpose the class is used, but unfortunatelly there is no such javadoc for the given class.

Since one really should not remove all docs from every class, can we at least make them optional?

Max mtg wrote:

jendave wrote:Note that if you remove the requirement for javadocs on some public methods you remove it for all public methods. Checkstyle does not determine how "trivial" a method is. Also, in your example you cite that the class should not have Javadoc as well. That is a very bad practice. All classes should have documentation explaining what they do. Just because there are examples of poor Javadocs does not mean that Javadocs should be tossed out.

Is there a problem about it? That looks really good.
So only non-trivial methods that really need docs will receive some.

The mentioned class does not need any javadocs at all, because it contains just a number of fields with their getters/setters. So a class with 13 fields turns into a 275-line mess. Would you agree with me, that understanding of this class will become easier if we just drop all javadocs from it? (because you may just scroll through the code and get the idea) The only javadoc worth keeping there is the one that would describe class's designation, that is when and for which purpose the class is used, but unfortunatelly there is no such javadoc for the given class.

Since one really should not remove all docs from every class, can we at least make them optional?

The class does indeed need javadocs. What is its purpose? How is it used? How has it changed over time? Javadocs are valuable so one does not have to look at the source code to understand the class. It is true that some classes benefit more from javadocs than others. The problem is if you make them optional, then almost certainly classes that do need Javadocs will not get them.

jendave wrote:The class does indeed need javadocs. What is its purpose? How is it used? How has it changed over time? Javadocs are valuable so one does not have to look at the source code to understand the class. It is true that some classes benefit more from javadocs than others. The problem is if you make them optional, then almost certainly classes that do need Javadocs will not get them.

Don't you see all of these docs are auto-generated? Why would you prefer auto-generated docs (which are bad and useless, consume space and distract attention) to none at all?

And why is our checkstyle prohibiting ternary operators?

Just compare

Code: Select all

        boolean isUniqueName;
        if (currentGameType.equals(GameType.Draft)) {
            isUniqueName = deckManager.isUniqueDraft(deckName);
        } else {
            isUniqueName = deckManager.isUnique(deckName);
        }

against

Code: Select all

        boolean isUniqueName = currentGameType == GameType.Draft ? deckManager.isUniqueDraft(deckName) : deckManager.isUnique(deckName);
       

Max mtg wrote:

jendave wrote:The class does indeed need javadocs. What is its purpose? How is it used? How has it changed over time? Javadocs are valuable so one does not have to look at the source code to understand the class. It is true that some classes benefit more from javadocs than others. The problem is if you make them optional, then almost certainly classes that do need Javadocs will not get them.

Don't you see all of these docs are auto-generated? Why would you prefer auto-generated docs (which are bad and useless, consume space and distract attention) to none at all?

I never said I prefer auto-generated docs to none at all. That is a false dilemma. I prefer Javadocs that contain useful information. As I said before, you are showing an example of poor Javadocs and proposing that we get rid of the requirement to have Javadocs at all.

Max mtg wrote:And why is our checkstyle prohibiting ternary operators?

Just compare

Code: Select all

        boolean isUniqueName;
        if (currentGameType.equals(GameType.Draft)) {
            isUniqueName = deckManager.isUniqueDraft(deckName);
        } else {
            isUniqueName = deckManager.isUnique(deckName);
        }

against

Code: Select all

        boolean isUniqueName = currentGameType == GameType.Draft ? deckManager.isUniqueDraft(deckName) : deckManager.isUnique(deckName);
       

Thanks. Fixed. r10314

jendave wrote:I never said I prefer auto-generated docs to none at all. That is a false dilemma. I prefer Javadocs that contain useful information. As I said before, you are showing an example of poor Javadocs and proposing that we get rid of the requirement to have Javadocs at all.

I do, because as it seems to me, we recieve poor javadocs because some are requied. If we lifted that requirement, I expect developers would write no javadocs instead of poor ones for trivial code and would write good ones, where they consider such docs indispensable.

Max mtg wrote:And why is our checkstyle prohibiting ternary operators?

Just compare

Code: Select all

        boolean isUniqueName;
        if (currentGameType.equals(GameType.Draft)) {
            isUniqueName = deckManager.isUniqueDraft(deckName);
        } else {
            isUniqueName = deckManager.isUnique(deckName);
        }

against

Code: Select all

        boolean isUniqueName = currentGameType == GameType.Draft ? deckManager.isUniqueDraft(deckName) : deckManager.isUnique(deckName);
       

The second set of code is not as clear as the first one. I would expect comments to explain it.. the first set of code is explicit and immediately apparent what it does. Also, if you wanted to add code into the if blocks at a later time, it's easy to do. I think ternary operators should be prohibited after all....

The percieved clearness of code might be due to personal habbits.
But look, when there is a long method body, such a reduced form is helping very much to get the idea of the rest.

Ternary opertor is not supposed to do anything besides choosing which branch to choose to assign to left part of expression.
If you want to add other code, just convert this back into an if-else clause.

Why should we convert from ternary to explicit if-block, when it would be easier to just use if-blocks in the first place?

Rob Cashwalker wrote:Why should we convert from ternary to explicit if-block, when it would be easier to just use if-blocks in the first place?

Sorry, easier for who?
Definetelly not for me. Choosing between writing 6 lines for an assignment-with-condition and 1 I surely choose the latter.