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

Generic SkipList implementation in Java

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

Problem

I am interested in implementing advanced data structures. One of them that tickled my fancy is the 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.