patternjavaMinor
Generic SkipList implementation in Java
Viewed 0 times
genericimplementationskiplistjava
Problem
I am interested in implementing advanced data structures. One of them that tickled my fancy is the
I wanted to know if there is anything to improve in my code.
The
The
```
/**
* File: SkipHeightGen.java
*/
import java.util.Random;
import java.util.ArrayList;
public class SkipHeightGen{
private Random rand;
private int maxLevel;
// Ideally the probability should be somewhere between
// 1/2, 1/3, 1/e
private double probability;
// For the different implementations of the
// SkipList, the initialLevel can either be 0(quad-node pointers)
// or 1(array version)
private int initLevel;
public SkipHeightGen(int initLevel, int maxLevel){
this(initLevel, maxLevel, 0.0);
}
public SkipHeightGen(int initLevel,
int maxLevel, double probability){
this.initLevel = initLev
SkipList.I wanted to know if there is anything to improve in my code.
The
SkipList node:/**
* SKNode.java
*/
import java.io.Serializable;
class SKNode> implements Serializable{
private SKNode up = null;
private SKNode down = null;
private SKNode next = null;
private SKNode prev = null;
private int printPosition = 0;
private T data;
SKNode(T data){ this.data = data;}
public void setUp(SKNode up){this.up = up;}
public void setDown(SKNode down){this.down = down;}
public void setNext(SKNode next){this.next = next;}
public void setPrev(SKNode prev){this.prev = prev;}
public void setPrintPosition(int printPosition){ this.printPosition = printPosition;}
public T getData(){return data;}
public SKNode getUp(){return up;}
public SKNode getDown(){return down;}
public SKNode getNext(){return next;}
public SKNode getPrev(){return prev;}
public int getPrintPosition(){ return printPosition;}
@Override
// To avoid NullPointerException, we check if object is null, if it is, print "null"
public String toString(){ return (data == null ? "null" : data.toString()); }
}The
SkipList height generator:```
/**
* File: SkipHeightGen.java
*/
import java.util.Random;
import java.util.ArrayList;
public class SkipHeightGen{
private Random rand;
private int maxLevel;
// Ideally the probability should be somewhere between
// 1/2, 1/3, 1/e
private double probability;
// For the different implementations of the
// SkipList, the initialLevel can either be 0(quad-node pointers)
// or 1(array version)
private int initLevel;
public SkipHeightGen(int initLevel, int maxLevel){
this(initLevel, maxLevel, 0.0);
}
public SkipHeightGen(int initLevel,
int maxLevel, double probability){
this.initLevel = initLev
Solution
if (levels >= height)
while (height <= levels)
buildEmptyLevel();Don't worry, a
while construct will check the condition before running. No need for the if here.private SKNode find(T data){
SKNode curPos = head;
// while(curPos != null) -- Possible alteration
/**
while(!curPos.toString().equals("null")
&& curPos.getData().compareTo(data) != 0){
if (curPos.getNext().getData().compareTo(data) > 0)
curPos = curPos.getDown();
else
curPos = curPos.getNext();
if (curPos == null) break;
}*/
while (true){
// Check to see you have not reached the end of the row
// and also check to see if you have not found a node either
// less than or equal to what you are looking for.
while(!(curPos.getNext().toString().equals("null")) &&
curPos.getNext().getData().compareTo(data) <= 0)
curPos = curPos.getNext();
// If node is found, move down a level below
if (curPos.getDown() != null)
curPos = curPos.getDown();
else
// On the last level, stop
break;
}
return curPos;
}This is unmaintainable. You've got dead code, comments explaining the obvious, lack of brackets...
You should remove the dead code.
You should clear away the comments that say obvious things like "and also check to see if you have not found a node either less than or equal to what you are looking for". Replace them with why you are doing what you are doing.
You should add braces to clarify the code.
Code Snippets
if (levels >= height)
while (height <= levels)
buildEmptyLevel();private SKNode<T> find(T data){
SKNode<T> curPos = head;
// while(curPos != null) -- Possible alteration
/**
while(!curPos.toString().equals("null")
&& curPos.getData().compareTo(data) != 0){
if (curPos.getNext().getData().compareTo(data) > 0)
curPos = curPos.getDown();
else
curPos = curPos.getNext();
if (curPos == null) break;
}*/
while (true){
// Check to see you have not reached the end of the row
// and also check to see if you have not found a node either
// less than or equal to what you are looking for.
while(!(curPos.getNext().toString().equals("null")) &&
curPos.getNext().getData().compareTo(data) <= 0)
curPos = curPos.getNext();
// If node is found, move down a level below
if (curPos.getDown() != null)
curPos = curPos.getDown();
else
// On the last level, stop
break;
}
return curPos;
}Context
StackExchange Code Review Q#133919, answer score: 3
Revisions (0)
No revisions yet.