Skip to main content
added 719 characters in body
Source Link
public class DLList<E extends Comparable<? super E>> 
public class DLList<E extends Comparable<? super E>> 

A better option would be to use a Comparator to determine the ordering of elements.

If you choose this route, your list would need to expose a constructor expecting a Comparator as a parameter.

public class DLList<E> { private final Comparator<? super E> comparator; } 
public class DLList<E> { private final Comparator<? super E> comparator; } 

Where Comparator<? super E> means that this comparator is capable of dialing with instances of type E or one of its super-typetypes.

If you choose this route, your list would need to expose a constructor expecting a Comparator as a parameter.

Optionally it might also expose a constructor which does not require a comparator, and in such a case while sorting elements they should be treated as comparable; i.e. you need to cast the values of nodes into a Comparable of raw type. That's one of the rare cases when use raw types is justifiable (but you have many others that are not, more on that below), and don't forget to apply annotation @SuppressWarnings("unchecked") while performing this cast.

Redundant Constructors / Initialization to null

No-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise.

  • No-args constructors (in classes that do not declare parameterized constructors), which initialize attributes to default values defined by the language specification (such as null for reference types) only create noise.
NodePair() { beg = null; end = null; } 
  • When a class has more than one constructor, you can call constructors from one another (so-called telescopic constructors). But, please, doesn't perform the automatic assignment to null manually. If a developer has chosen to use a certain technology (language, framework), it should be well-understood and trusted.

For instanceSo instead of this:

NodePair() { beg = null; end = null; } 
DLNode() { next = null; prev = null; element = null; } DLNode(E e) { next = null; prev = null; element = e; } 

I'll suggest the following:

DLNode() { this(null); } DLNode(E e) { element = e; } 
public class DLList<E extends Comparable<? super E>> 

A better option would be to use a Comparator to determine the ordering elements.

If you choose this route, your list would need to expose a constructor expecting a Comparator as a parameter.

public class DLList<E> { private final Comparator<? super E> comparator; } 

Where Comparator<? super E> means that this comparator is capable of dialing with instances of type E or one of its super-type.

Optionally it might also expose a constructor which does not require a comparator, and in such a case while sorting elements they should be treated as comparable; i.e. you need to cast the values of nodes into a Comparable of raw type. That's one of the rare cases when use raw types is justifiable (but you have many others that are not, more on that below), and don't forget to apply annotation @SuppressWarnings("unchecked") while performing this cast.

Redundant Constructors

No-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise.

For instance:

NodePair() { beg = null; end = null; } 
public class DLList<E extends Comparable<? super E>> 

A better option would be to use a Comparator to determine the ordering of elements.

public class DLList<E> { private final Comparator<? super E> comparator; } 

Where Comparator<? super E> means that this comparator is capable of dialing with instances of type E or one of its super-types.

If you choose this route, your list would need to expose a constructor expecting a Comparator as a parameter.

Optionally it might also expose a constructor which does not require a comparator, and in such a case while sorting elements they should be treated as comparable; i.e. you need to cast the values of nodes into a Comparable of raw type. That's one of the rare cases when use raw types is justifiable (but you have many others that are not, more on that below), and don't forget to apply annotation @SuppressWarnings("unchecked") while performing this cast.

Redundant Constructors / Initialization to null

  • No-args constructors (in classes that do not declare parameterized constructors), which initialize attributes to default values defined by the language specification (such as null for reference types) only create noise.
NodePair() { beg = null; end = null; } 
  • When a class has more than one constructor, you can call constructors from one another (so-called telescopic constructors). But, please, doesn't perform the automatic assignment to null manually. If a developer has chosen to use a certain technology (language, framework), it should be well-understood and trusted.

So instead of this:

DLNode() { next = null; prev = null; element = null; } DLNode(E e) { next = null; prev = null; element = e; } 

I'll suggest the following:

DLNode() { this(null); } DLNode(E e) { element = e; } 
added 1279 characters in body
Source Link

Class DLList representing your doubly-linked list implementation should be public and its attributes private.

Your list is brokenbroken because it allows adding elements that are non-comparable. And then when it comes to sorting, it stipulates that values should implement Comparable.

  1. You can mandate upfront that list-elements should be comparable by changing the class declaration:
  • You can mandate upfront that list-elements should be comparable by changing the class declaration:

Where Comparable<? super E> means that either E or one of its super-types implements Comparable (which is more flexible than E extends Comparable<E>, i.e. E type itself implements comparable, and not inherits implementation).Where Comparable<? super E> means that either E or one of its super-types implements Comparable (which is more flexible than E extends Comparable<E>, i.e. E type itself implements comparable, and not inherits implementation).

  1. Use Comparator

A better option would be to use a ComparatorComparator to determine the ordering elements.

Domain typesIf you choose this route, your list would need to expose a constructor expecting a Comparator as a parameter.

public class DLList<E> { private final Comparator<? super E> comparator; } 

Where Comparator<? super E> means that this comparator is capable of dialing with instances of type E or one of its super-type.

Optionally it might also expose a constructor which does not require a comparator, and in such a case while sorting elements they should be treated as comparable; i.e. you need to cast the values of nodes into a Comparable of raw type. That's one of the rare cases when use raw types is justifiable (but you have many others that are not, more on that below), and don't forget to apply annotation @SuppressWarnings("unchecked") while performing this cast.


Regardless of which of the two strategies listed above you choose, method merge() should not declare its own generic type parameter before its return type.

No-args constructorsNo-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise. 

You don't need them, Java-compiler will autogenerateautogenerate them for you.

E.g.For instance:

Raw TypesRaw Types

You're using raw types in many places of the code without good reason.

  • DLNode<E> node = new DLNode(element); -> DLNode<E> node = new DLNode<>(element);

  • list = new <E> DLNode(); -> list = new DLNode<>();

The Java compiler will spot all such cases for you and will issue a warning for each of them.

Naming

  • In Java, it's not considered a good practice to skimp on characters. Especially, when it comes to publicly exposed names, such as the names of public types and methods, they clearly communicate the purpose to the user.

For instance, DLList is not a very descriptive name, DL isn't a widely-used abbreviation, and it's discouraged by the Java language naming convention

Try to keep your class names simple and descriptive. Use whole words-avoid acronyms and abbreviations (unless the abbreviation is much more widely used than the long form, such as URL or HTML).

A proper convention-aligned name would be DoublyLinkedList.

  • Also according to the naming convention method names should be in camel-case: push_front() -> pushFront().

Class DLList representing your doubly-linked list implementation should be public.

Your list is broken because it allows adding elements that are non-comparable. And then when it comes to sorting, it stipulates that values should implement Comparable.

  1. You can mandate upfront that list-elements should be comparable by changing the class declaration:

Where Comparable<? super E> means that either E or one of its super-types implements Comparable (which is more flexible than E extends Comparable<E>, i.e. E type itself implements comparable, and not inherits implementation).

  1. Use Comparator

A better option would be to use Comparator to determine the ordering elements.

Domain types

No-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise. You don't need them, Java-compiler will autogenerate them for you.

E.g.:

Raw Types

You're using raw types in many places of the code.

  • DLNode<E> node = new DLNode(element); -> DLNode<E> node = new DLNode<>(element);

  • list = new <E> DLNode(); -> list = new DLNode<>();

Class DLList representing your doubly-linked list implementation should be public and its attributes private.

Your list is broken because it allows adding elements that are non-comparable. And then when it comes to sorting, it stipulates that values should implement Comparable.

  • You can mandate upfront that list-elements should be comparable by changing the class declaration:

Where Comparable<? super E> means that either E or one of its super-types implements Comparable (which is more flexible than E extends Comparable<E>, i.e. E type itself implements comparable, and not inherits implementation).

A better option would be to use a Comparator to determine the ordering elements.

If you choose this route, your list would need to expose a constructor expecting a Comparator as a parameter.

public class DLList<E> { private final Comparator<? super E> comparator; } 

Where Comparator<? super E> means that this comparator is capable of dialing with instances of type E or one of its super-type.

Optionally it might also expose a constructor which does not require a comparator, and in such a case while sorting elements they should be treated as comparable; i.e. you need to cast the values of nodes into a Comparable of raw type. That's one of the rare cases when use raw types is justifiable (but you have many others that are not, more on that below), and don't forget to apply annotation @SuppressWarnings("unchecked") while performing this cast.


Regardless of which of the two strategies listed above you choose, method merge() should not declare its own generic type parameter before its return type.

No-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise. 

You don't need them, Java-compiler will autogenerate them for you.

For instance:

Raw Types

You're using raw types in many places of the code without good reason.

  • DLNode<E> node = new DLNode(element); -> DLNode<E> node = new DLNode<>(element);

  • list = new <E> DLNode(); -> list = new DLNode<>();

The Java compiler will spot all such cases for you and will issue a warning for each of them.

Naming

  • In Java, it's not considered a good practice to skimp on characters. Especially, when it comes to publicly exposed names, such as the names of public types and methods, they clearly communicate the purpose to the user.

For instance, DLList is not a very descriptive name, DL isn't a widely-used abbreviation, and it's discouraged by the Java language naming convention

Try to keep your class names simple and descriptive. Use whole words-avoid acronyms and abbreviations (unless the abbreviation is much more widely used than the long form, such as URL or HTML).

A proper convention-aligned name would be DoublyLinkedList.

  • Also according to the naming convention method names should be in camel-case: push_front() -> pushFront().
added 1279 characters in body
Source Link

Code Structure / Encapsulation

Class DLList representing your doubly-linked list implementation should be public.

Auxiliary classes should be encapsulated within the list as nested private static since they are only supposed to be used internally by the list.

External code should have no notion of list-nodes, list should be a designed as a user-friendly abstraction instead of leaking its implementation details.

Comparable

Your list is broken because it allows adding elements that are non-comparable. And then when it comes to sorting, it stipulates that values should implement Comparable.

This issue can be resolved in two ways.

  1. You can mandate upfront that list-elements should be comparable by changing the class declaration:
public class DLList<E extends Comparable<? super E>> 

Where Comparable<? super E> means that either E or one of its super-types implements Comparable (which is more flexible than E extends Comparable<E>, i.e. E type itself implements comparable, and not inherits implementation).

  1. Use Comparator

Since Comparable is supposed to be implemented only by types that have natural ordering, demanding that elements should be comparable is a very strict constraint that will limit the usage of the list.

For example, it's almost never the case when domain types have natural ordering (there's no unified widely recognized way to compare students, employees, books, etc.).

A better option would be to use Comparator to determine the ordering elements.

Domain types

Redundant Constructors

No-args constructors that only initialize attributes to default values defined by the language specification defined by the language specification (such as null for reference types) only create noise. You don't need them, Java-compiler will autogenerate them for you.

E.g.:

NodePair() { beg = null; end = null; } 

Raw Types

You're using raw types in many places of the code.

  • NodePair should be declared as generic, as well as its fields.

  • Generic type variables are missing in the parameters type declarations in the splice() and sortr() methods (not a very discriptivedescriptive name, by the way).

Generic types should be provided while instantiating a generic class. Either explicitly (e.g. new DLNode<E>()), or through the type-inference mechanism by using so-called diamond operator which is more convenient.

  • DLNode<E> node = new DLNode(element); -> DLNode<E> node = new DLNode<>(element);

  • list = new <E> DLNode(); -> list = new DLNode<>();

Code Structure / Encapsulation

Class DLList representing your doubly-linked list implementation should be public.

Auxiliary classes should be encapsulated within the list as nested private static since they are only supposed to be used internally by the list.

External code should have no notion of list-nodes, list should be a designed as a user-friendly abstraction instead of leaking its implementation details.

Redundant Constructors

No-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise. You don't need them, Java-compiler will autogenerate them for you.

E.g.:

NodePair() { beg = null; end = null; } 

Raw Types

You're using raw types in many places of the code.

  • NodePair should be declared as generic, as well as its fields.

  • Generic type variables are missing in the parameters type declarations in the splice() and sortr() methods (not a very discriptive name by the way).

Generic types should be provided while instantiating a generic class. Either explicitly (e.g. new DLNode<E>()), or through the type-inference mechanism by using so-called diamond operator which is more convenient.

  • DLNode<E> node = new DLNode(element); -> DLNode<E> node = new DLNode<>(element);

  • list = new <E> DLNode(); -> list = new DLNode<>();

Code Structure / Encapsulation

Class DLList representing your doubly-linked list implementation should be public.

Auxiliary classes should be encapsulated within the list as nested private static since they are only supposed to be used internally by the list.

External code should have no notion of list-nodes, list should be a designed as a user-friendly abstraction instead of leaking its implementation details.

Comparable

Your list is broken because it allows adding elements that are non-comparable. And then when it comes to sorting, it stipulates that values should implement Comparable.

This issue can be resolved in two ways.

  1. You can mandate upfront that list-elements should be comparable by changing the class declaration:
public class DLList<E extends Comparable<? super E>> 

Where Comparable<? super E> means that either E or one of its super-types implements Comparable (which is more flexible than E extends Comparable<E>, i.e. E type itself implements comparable, and not inherits implementation).

  1. Use Comparator

Since Comparable is supposed to be implemented only by types that have natural ordering, demanding that elements should be comparable is a very strict constraint that will limit the usage of the list.

For example, it's almost never the case when domain types have natural ordering (there's no unified widely recognized way to compare students, employees, books, etc.).

A better option would be to use Comparator to determine the ordering elements.

Domain types

Redundant Constructors

No-args constructors that only initialize attributes to default values defined by the language specification (such as null for reference types) only create noise. You don't need them, Java-compiler will autogenerate them for you.

E.g.:

NodePair() { beg = null; end = null; } 

Raw Types

You're using raw types in many places of the code.

  • NodePair should be declared as generic, as well as its fields.

  • Generic type variables are missing in the parameters type declarations in the splice() and sortr() methods (not a very descriptive name, by the way).

Generic types should be provided while instantiating a generic class. Either explicitly (e.g. new DLNode<E>()), or through the type-inference mechanism by using so-called diamond operator which is more convenient.

  • DLNode<E> node = new DLNode(element); -> DLNode<E> node = new DLNode<>(element);

  • list = new <E> DLNode(); -> list = new DLNode<>();

Source Link
Loading