Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##Imports:

Imports:

import java.util.*;

NEVER do this. Look how many classes you got now. This is ineffective and needlessly clutters your IDE's "do what I want button"-suggestions.

The fun thing is, in your main class you do it the other way round:

java.util.Arrays.sort(data, new java.util.Comparator<Object>() {

why not:

import java.util.Arrays;
import java.util.Comparator;
import java.util.NoSuchElementException;
//Later:
Arrays.sort(data, new Comparator<Object>()) {

This is shorter, does exactly the same and allows your ClassLoader to preload the classes needed --> possible performance improvements

##Queue<E> and Scoping

Queue<E> and Scoping

While we're in the java.util package.. You can find an interface there.. Queue<E>. This is something you can program against. To prevent a name-conflict I'd rename your implementation to MyQueue or something similar.

You class would then begin with the following:

public class MyQueue implements Queue<Integer> {
 private Integer[] data;
 private int head;
 private int tail;

Here I also made some changes to your code.
Why? In short: You should restrict the visibility of members as much as possible. With your code, classes in the same package as your Queue could do:

Queue queue = new Queue(); 
queue.data[0] = Integer.MAX_VALUE;

This is not good. noone should be messing with the state of your class. That's why these members should be private (or maximally protected), but definitely not package-private which is the default scope.

same goes for your head and tail.

##Imports:

import java.util.*;

NEVER do this. Look how many classes you got now. This is ineffective and needlessly clutters your IDE's "do what I want button"-suggestions.

The fun thing is, in your main class you do it the other way round:

java.util.Arrays.sort(data, new java.util.Comparator<Object>() {

why not:

import java.util.Arrays;
import java.util.Comparator;
import java.util.NoSuchElementException;
//Later:
Arrays.sort(data, new Comparator<Object>()) {

This is shorter, does exactly the same and allows your ClassLoader to preload the classes needed --> possible performance improvements

##Queue<E> and Scoping

While we're in the java.util package.. You can find an interface there.. Queue<E>. This is something you can program against. To prevent a name-conflict I'd rename your implementation to MyQueue or something similar.

You class would then begin with the following:

public class MyQueue implements Queue<Integer> {
 private Integer[] data;
 private int head;
 private int tail;

Here I also made some changes to your code.
Why? In short: You should restrict the visibility of members as much as possible. With your code, classes in the same package as your Queue could do:

Queue queue = new Queue(); 
queue.data[0] = Integer.MAX_VALUE;

This is not good. noone should be messing with the state of your class. That's why these members should be private (or maximally protected), but definitely not package-private which is the default scope.

same goes for your head and tail.

Imports:

import java.util.*;

NEVER do this. Look how many classes you got now. This is ineffective and needlessly clutters your IDE's "do what I want button"-suggestions.

The fun thing is, in your main class you do it the other way round:

java.util.Arrays.sort(data, new java.util.Comparator<Object>() {

why not:

import java.util.Arrays;
import java.util.Comparator;
import java.util.NoSuchElementException;
//Later:
Arrays.sort(data, new Comparator<Object>()) {

This is shorter, does exactly the same and allows your ClassLoader to preload the classes needed --> possible performance improvements

Queue<E> and Scoping

While we're in the java.util package.. You can find an interface there.. Queue<E>. This is something you can program against. To prevent a name-conflict I'd rename your implementation to MyQueue or something similar.

You class would then begin with the following:

public class MyQueue implements Queue<Integer> {
 private Integer[] data;
 private int head;
 private int tail;

Here I also made some changes to your code.
Why? In short: You should restrict the visibility of members as much as possible. With your code, classes in the same package as your Queue could do:

Queue queue = new Queue(); 
queue.data[0] = Integer.MAX_VALUE;

This is not good. noone should be messing with the state of your class. That's why these members should be private (or maximally protected), but definitely not package-private which is the default scope.

same goes for your head and tail.

Source Link
Vogel612
  • 25.5k
  • 7
  • 59
  • 141

##Imports:

import java.util.*;

NEVER do this. Look how many classes you got now. This is ineffective and needlessly clutters your IDE's "do what I want button"-suggestions.

The fun thing is, in your main class you do it the other way round:

java.util.Arrays.sort(data, new java.util.Comparator<Object>() {

why not:

import java.util.Arrays;
import java.util.Comparator;
import java.util.NoSuchElementException;
//Later:
Arrays.sort(data, new Comparator<Object>()) {

This is shorter, does exactly the same and allows your ClassLoader to preload the classes needed --> possible performance improvements

##Queue<E> and Scoping

While we're in the java.util package.. You can find an interface there.. Queue<E>. This is something you can program against. To prevent a name-conflict I'd rename your implementation to MyQueue or something similar.

You class would then begin with the following:

public class MyQueue implements Queue<Integer> {
 private Integer[] data;
 private int head;
 private int tail;

Here I also made some changes to your code.
Why? In short: You should restrict the visibility of members as much as possible. With your code, classes in the same package as your Queue could do:

Queue queue = new Queue(); 
queue.data[0] = Integer.MAX_VALUE;

This is not good. noone should be messing with the state of your class. That's why these members should be private (or maximally protected), but definitely not package-private which is the default scope.

same goes for your head and tail.

lang-java

AltStyle によって変換されたページ (->オリジナル) /