HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Page Enum with overridden methods for the first and last members

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lastthewithenummethodsfirstpageformembersand

Problem

I am managing the changing of views in an Android app using this enum and associated methods. I am particularly interested in the Page enum's use of overriding previous() and next() for the first and last elements.

  • Is this readable?



  • Is this idiomatic?



  • Is there a preferable data structure?



  • Are there general stylistic improvements that could be made?



public enum Page {
  INTRO { @Override public Page previous() {return this; } },
  PUBLIC_PROFILE, PRIVATE_PROFILE, SOCIAL_ACCOUNTS, LOOPS, INVITE,
  FINISHED { @Override public Page next() { return this; }; };

  public Page next() {
    return values()[ordinal()+1];
  }

  public Page previous() {
    return values()[ordinal()-1];
  }
}

private Page page = Page.INTRO;

private void setPage(Page page) { ... }

private void nextPage() {
  page = page.next();
  setPage(page);
}

private void previousPage() {
  page = page.previous();
  setPage(page);
}

Solution

Interesting question. Overriding methods in enum instances is not something I have encountered (that I recall). This is not to say it's a bad thing, just not very common. (P.S. having just claimed not to have encountered enum overrides, I see it is used in this answer given shortly before you asked this very question

The concept is something I have solved using different mechanisms before, but I can understand why having the override makes sense to you.

What I don't understand is why you did not just use a regular bound-clipping system. Why do you need the overrides at all?

Consider these methods:

public Page next() {
    if (ordinal() == values().length - 1) {
        return this;
    }
    return values()[ordinal() + 1];
  }

  public Page previous() {
    if (ordinal() == 0) {
        return this;
    }
    return values()[ordinal() - 1];
  }


There is no reason to override them at all.

On the other hand, if you do override them, then please use a standard indentation for the override methods. Cramming it all on one line is not helpful:

public enum Page {
    INTRO {
        @Override
        public Page previous() {
            return this;
        }
    },
    PUBLIC_PROFILE,
    PRIVATE_PROFILE,
    SOCIAL_ACCOUNTS,
    LOOPS,
    INVITE,
    FINISHED {
        @Override
        public Page next() {
            return this;
        };
    };

Code Snippets

public Page next() {
    if (ordinal() == values().length - 1) {
        return this;
    }
    return values()[ordinal() + 1];
  }

  public Page previous() {
    if (ordinal() == 0) {
        return this;
    }
    return values()[ordinal() - 1];
  }
public enum Page {
    INTRO {
        @Override
        public Page previous() {
            return this;
        }
    },
    PUBLIC_PROFILE,
    PRIVATE_PROFILE,
    SOCIAL_ACCOUNTS,
    LOOPS,
    INVITE,
    FINISHED {
        @Override
        public Page next() {
            return this;
        };
    };

Context

StackExchange Code Review Q#91304, answer score: 3

Revisions (0)

No revisions yet.