Skip to main content
added 445 characters in body; added 88 characters in body; edited body
Source Link
psaxton
  • 902
  • 5
  • 9

I agree with Sleiman's review, so I'll skip those points.

I'm not a fan of your Constants class.

  • being a final class prevents any other implementations. If you decide later to change the window size or limits, you can't.
  • the members being public static final int prevents any configuration at run time. Maybe you only care about 500 messages now, but what happens when that important message is 501 away?

I would recommend an interface with read-only properties (getters, no setters) and have your final class implement that interface and return its constant values.

I'm also not a fan of your Log class.

  • it violates SRP. Is it a logger or a ring buffer? I would recommend using an existing ring buffer implementation for your storage in place of a List, or rolling your own if this is academic.
  • there are already logging mechanisms built into the base Java libraries as well as the excellent log4j third party library. When learning new languages it is important to understand what the language provides.

I'm not a fan of your Constants class.

  • being a final class prevents any other implementations. If you decide later to change the window size or limits, you can't.
  • the members being public static final int prevents any configuration at run time. Maybe you only care about 500 messages now, but what happens when that important message is 501 away?

I would recommend an interface with read-only properties (getters, no setters) and have your final class return its constant values.

I agree with Sleiman's review, so I'll skip those points.

I'm not a fan of your Constants class.

  • being a final class prevents any other implementations. If you decide later to change the window size or limits, you can't.
  • the members being public static final int prevents any configuration at run time. Maybe you only care about 500 messages now, but what happens when that important message is 501 away?

I would recommend an interface with read-only properties (getters, no setters) and have your final class implement that interface and return its constant values.

I'm also not a fan of your Log class.

  • it violates SRP. Is it a logger or a ring buffer? I would recommend using an existing ring buffer implementation for your storage in place of a List, or rolling your own if this is academic.
  • there are already logging mechanisms built into the base Java libraries as well as the excellent log4j third party library. When learning new languages it is important to understand what the language provides.
Source Link
psaxton
  • 902
  • 5
  • 9

I'm not a fan of your Constants class.

  • being a final class prevents any other implementations. If you decide later to change the window size or limits, you can't.
  • the members being public static final int prevents any configuration at run time. Maybe you only care about 500 messages now, but what happens when that important message is 501 away?

I would recommend an interface with read-only properties (getters, no setters) and have your final class return its constant values.