views:

321

answers:

7

I need to create a phoneBook. Thanks to a form, I can retrieve a person's data. I need to use objects for that purpose. I created a phoneBook() object with the help of a method that enables to add a person in the phoneBook.
I decided (it wasn't asked for though) to divide the "person" concept in 2, which results in a "Person" object and an "AddressPerson" object (a same person can have two houses: "My tailor is rich!" :-))

Is it a good way to declare the Person object?
Can we create a Person object without address and add it later on like I did with the "Person2" object?

If someone could help me, I'd be very obliged. Thank you very much in advance!"

function phoneBook(){
  this.Liste = new Array(); 
}

phoneBook.prototype.Add = function(){
   Liste.push(new Person(aLastName,aFirstName,aAddress));
}

function Person(aLastName,aFirstName,aAdd){
  this.LastName   = aLastName;
  this.FirstName  = aFirstName;
  this.Address = 
    new AddressPerson(aAdd.Street,aAdd.CP,aAdd.Town,aAdd.NumTel,aAdd.Email);
}

function Person2(aLastName,aFirstName){
  this.LastName   = aLastName;
  this.FirstName  = aFirstName;
  this.Address = 'unknow';
}

function AddressPerson(aStreet,aCP,aTown,aNumTel,aEmail){
  this.Street = aStreet;
  this.CP    = aCP;
  this.Town = aTown;
  this.NumTel = aNumTel;
  this.Email= aEmail;
}
+4  A: 

Some suggestions:

  • Change your class (PhoneBook) to be Capitalized, and your methods/properties (lastName, add()) to be lower-case
  • The 'a' prefix on method parameters is not needed, since in Javascript this is never implicitly used.
  • Methods should take objects as parameters. For example, add() should be add(person) where person is a pre-constructed Person object.
  • Why do you need Person2? It seems redundant.
  • In constructor for Person, you copy every field of the Address. Generally just doing this.address = address would be OK. But if you want to ensure every Person has its own instance of Address, provide a clone() method on Address.
levik
+1  A: 

Take a look at JSLint. This work sort'a like W3C's Markup Validator.

JSLint is created by Douglas Crockford--a Yahoo! JavaScript evangelist and the dude who invented JSON.

What JSLint is all about:

"JSLint is a JavaScript program that looks for problems in JavaScript programs.

When C was a young programming language, there were several common programming errors that were not caught by the primitive compilers, so an accessory program called lint was developed which would scan a source file, looking for problems.

As the language matured, the definition of the language was strengthened to eliminate some insecurities, and compilers got better at issuing warnings. lint is no longer needed.

JavaScript is a young-for-its-age language. It was originally intended to do small tasks in webpages, tasks for which Java was too heavy and clumsy. But JavaScript is a very capable language, and it is now being used in larger projects. Many of the features that were intended to make the language easy to use are troublesome for larger projects. A lint for JavaScript is needed: JSLint, a JavaScript syntax checker and validator.

JSLint takes a JavaScript source and scans it. If it finds a problem, it returns a message describing the problem and an approximate location within the source. The problem is not necessarily a syntax error, although it often is. JSLint looks at some style conventions as well as structural problems. It does not prove that your program is correct. It just provides another set of eyes to help spot problems.

JSLint defines a professional subset of JavaScript, a stricter language than that defined by Edition 3 of the ECMAScript Language Specification. The subset is related to recommendations found in Code Conventions for the JavaScript Programming Language.

JavaScript is a sloppy language, but inside it there is an elegant, better language. JSLint helps you to program in that better language and to avoid most of the slop."

roosteronacid
A: 

There are some things you can do for more succinct code, as well as following today's accepted best practice.

You could change this line

  this.Liste = new Array();

to

  this.Liste = []; // shorthand for creating an array. You can do the same with an object with {}

I generally use JSON objects for this sort of thing. Douglas Crockford's JavaScript is a very useful resource.

alex
A: 

Thanks a lot
I changed my code like that :

function PhoneBook(){<br>
    this.liste = [];<br>
}

PhoneBook.prototype.add = function(person){<br>
   this.liste.push(person);<br>
}

function Person(lastName,firstName,address){<br>
    this.lastName   = lastName;<br>
    this.firstName  = firstName;<br>
    this.address = address;<br>
}

function AddressPerson(street,cp,town,numTel,email){<br>
    this.street = street;<br<
    this.cp     = cp;<br>
    this.town   = town;<br>
    this.numTel = numTel;<br>
    this.email  = email;<br>
}

// tests for the others :-))

var phone = new PhoneBook();
alert(phone.liste.length);
var person = new Person(
    "aaaaaa",
    "bbbbbbb",
    new AddressPerson("zzzzz","87","rrrrr","22222","[email protected]")
);
alert(person.address.street);
phone.add(person);
alert(phone.liste.length);
alert(phone.liste[0].address.numTel);

But I don't know how to do this, which Levik said in his answer:

But if you want to ensure every Person has its own instance of Address, provide a clone() method on Address

OUPS!function AddressPerson(street,cp,town,numTel,email){<br>this.street = street;<br> this.cp= cp;<br> this.town = town;<br>this.numTel = numTel;<br>this.email = email;<br>}
Without <br> tag : Sorry , sorry !!function AddressPerson(street,cp,town,numTel,email){ this.street = street; this.cp = cp; this.town = town; this.numTel = numTel; this.email = email;}
OUPS! function AddressPerson(street,cp,town,numTel,email){ this.street = street; this.cp = cp; this.town = town; this.numTel = numTel; this.email = email;}
hi gmatu - you can edit your answer by clicking the "edit" link just above the comments.
nickf
A: 

Here's my suggestion:

function PhoneBook( person /*, ... */ ){
    this._liste = [];
}

PhoneBook.prototype.add = function(){
    var l = arguments.length;
    while(l--){
     this._liste.unshift( arguments[l] );
    }
}

function Person( p ){
    this.lastName   = p.lastName;
    this.firstName = p.firstName;
    this.address  = p.address === 'unknown' ? new Address( p.address ) : 'unknown';
}

function Address( a ){
    this.street = a.street;
    this.CP     = a.CP;
    this.town  = a.town;
    this.numTel = a.numTel;
    this.email = a.email;
}
thanks for your answer too but sorry it's 3h30am for me I will test that tomorrow!
I don't think that changing the Person and Address constructors to accept an object is an improvement. That just makes construction of those objects unnecessarily complex IMO. Also, I think your add function will miss the 0th element of the arguments array?
harto
A: 

Data model suggestions:

  • Rename AddressPerson to Address. You could hypothetically pass Address objects around that don't correspond to a Person.

  • Consider moving the email field into Person - as you mentioned, you might have multiple people at one address (they might not necessarily share email addresses).

  • Pass a fully initialised Address into the Person constructor.

  • Remove Person2 function, and make address an optional attribute of Person. E.g. assignment becomes this.address = (address === undefined) ? "unknown" : address;

From a style/convention perspective, levik's response is good - I endorse those suggestions.

harto
A: 

Commenting on my own post comment, before I registered:

@harto - I don't think that changing the Person and Address constructors to accept an object is an improvement. That just makes construction of those objects unnecessarily complex IMO. Also, I think your add function will miss the 0th element of the arguments array?

I do think that it is good to change the constructors, it allows automatic cloning for one and, if your using it, allows creation of object directly from JSON. Anyway, plenty of applications. If needed, you could always have an arguments length verification and branch on the proper behavior.

As for the the add function, it won't miss the 0th element because it is post-decremented which mean that the expression returns the value before the decrement. It is the fastest loop you can devise in javascript.

Laurent Villeneuve
Quite right on the loop index, my mistake :)
harto