0
\$\begingroup\$

The purpose of the data-structure is to manage person-objects. Persons can be added at any time. But the order can't be changed afterward.

Only the deletion of a person is possible. Represents the case of cancellation.

The whole thing is intended to be as simple as possible.

Nevertheless: I'm thinking about an additional get-method which allows to retrieve a person by a key (e.g. last name). Currently you can get a person only via the position in the list.

But before I go on with programming I would like my code to be reviewed.

Therefore: Any feedback welcomed. :)

// ------ The actual list -----------------
  
/**
  * List-constructor
  */
function List() {
  this.first = null;
  this.last = null;
  this.size = 0;
}

/** 
  * ListItem-contructor
  * @param {Any data} data
  */
List.prototype.ListItem = function(data) {
  this.data = data || null;
  this.next = null;
};

/**
  *  Appends data to the list.
  *  @param {Any data} data
  */
List.prototype.appendItem = function(data) {
  var newItem = new this.ListItem(data);

  if (!this.first) {
    this.first = newItem;
    this.last = newItem;
  } else {
    this.last.next = newItem;
    this.last = newItem;
  }

  this.size++;
};

/**
  * Retrieves the item at a given position.
  * @param {Number} position
  * @param {Boolean} onlyData
  * @returns {Object | AnyData}
  * @throws Throws error if the position parameter is invalid.
  * @throws Throws error if position if greater then list-size.
  */
List.prototype.getItemAtPosition = function(position, onlyData) {
  var ret;
  var i;

  if (typeof position !== 'number' || isNaN(position) || position <= 0) {
    throw new Error('Unallowed index-range.');
  } else if (position > this.size) {
    throw new Error(
      'Assigned position greater then list-size. ' +
      'Check position-parameter!');
  } else if (position === this.size) {
    ret = this.last;
  } else {	  
    ret = this.first;
    i++; 

    for (i = 1; i < position; i++) {
      ret = ret.next; 
    }
  }	 

  if (onlyData) {
    return ret.data;
  } else {
    return ret;
  }	  
};

/**
  * Removes an item from the list.
  * @param {Number} position
  * @returns {AnyData} The data of the removed item.
  * @throws Throws error if the position parameter is invalid.
  * @throws Throws error if position if greater then list-size.
  */
List.prototype.removeItemAtPosition = function(position) {
  var ret; 
  var before;

  position = +position;

  if (typeof position !== 'number' || isNaN(position) || position <= 0) {
    throw new Error('Assigned position-parameter invalid.');
  } else if (position > this.size) {
    throw new Error(
      'Assigned position greater then list-size. ' +
      'Check position-parameter!');
  }
  // Get the item BEFORE the item to delete.
  before = this.getItemAtPosition(position - 1, false);
  ret = before.next; // The next item is the item toDelete.
  // Change reference. The to-delete item is now not longer referenced.
  before.next = ret.next;
  // If the toDelete-item is the current last item of
  // the list: There would be a reference left.
  if (position === this.size) {
    this.last = before; // Therfore: Overwrite it!
  }

  this.size--;

  return ret.data;
};
// ------ The actual list -----------------

// ------ TESTS Start -------------
l = console.log;
var i;
var list = new List();
var persons = [];
var removed; 
// -- Test-data -------------
for (i = 1; i <= 10; i++) {
  list.appendItem( 'Item-number-' + i );
}
// -- Test-data -------------

for (i = 1; i <= list.size; i++) {
  try {
    persons.push(list.getItemAtPosition(i, true));
  } catch (e) {
    persons.push(e.message);
  }
}

l(' -- Display the list -- ');
for (i in persons) {
  l('%s: %s', i, persons[i]);
}

l('\n -- Removing items -- ');

removed = list.removeItemAtPosition(2);
l('Removed: ' + removed); // Expected: 'second'

removed = list.removeItemAtPosition(5);
l('Removed: ' + removed); // Expected: 'sixth'

removed = list.removeItemAtPosition(3);
l('Removed: ' + removed);  // Expected: 'fourth'

persons.length = 0;

for (i = 1; i <= list.size; i++) {
  try {
    persons.push(list.getItemAtPosition(i, true));
  } catch (e) {
    persons.push(e.message);
  }
}

l('\n -- Display the modified list -- ');
// Expected: 'first', 'third', 'fifth'
for (i in persons) {
  l('%s: %s', i, persons[i]);
}
// ------ TESTS End -------------

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Before anything else, JavaScript has arrays that act like an array, a stack, and a queue, among other things that it can do. So there's literally little reason to reproduce a fundamental data structure when there are native built-ins that do it for you.

List -> [] (array)
appendItem -> array.push(x)
getItemAtPosition -> array[n]
removeItemAtPosition -> array.splice(n, 1);

Anyways...

List.prototype.ListItem = function(data) {

This looks to me like a list item constructor. But I would suggest not putting it on the prototype. There's little reason for it to be there. Suggesting you make it "static" instead by assigning it as a property of List (In JS, everything is an object, including functions).

if (onlyData) {
  return ret.data;
} else {
  return ret;
}

Functions that try to do too many things can be a future problem. In this case, getItemAtPosition is trying to return 2 different things depending on some argument. Suggesting you split off the version that returns the data into a separate function that calls getItemAtPosition and extracts the data (something like getItemDataAtPosition). This way, you're not caught off guard.

\$\endgroup\$
1
  • \$\begingroup\$ I was indeed not sure where to attach the ListItem-constructor. Thought about attaching it to the list-constructor together with first, last, size. Thanks for your constructive feedback. \$\endgroup\$ Commented May 4, 2016 at 13:01

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.