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
\$\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\$ May 4 '16 at 13:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.