Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a test which looks like this:

  • create a base class Human
  • Human class needs to have a method Talk
  • the Human class needs to have to descendant class Man and Woman
  • both Man and Woman should have their own Talk method (basically rewrite the method)
  • the Man class should have a private property _foo
  • Also it should have a method getInfo (which needs to be an ajax call, an log the response)
  • I need to make 1000 instances of the Women class in the window namespace (i mean global)
  • on document.body single click a random women should call the talk method
  • on document.body double click the Man getInfo method should be called

So I now there are plenty of stuff but please bear with me. I already wrote the code, i would just like someone to check it out say an opinion.

I'm very grateful for any hopeful response. So here's the code:

function Human(){};

Human.prototype.talk = function(){
    console.log('Make an Human sound');
}

function Woman(){
    Human.call(this);
}


Woman.prototype = Object.create(Human.prototype);
Woman.prototype.constructor = Woman;

Woman.prototype.talk = function(){
    console.log('Miau');
}

function Man(){
    var foo = 10;
    Human.call(this);
}


Man.prototype = Object.create(Human.prototype);
Man.prototype.constructor = Man;

Man.prototype.talk = function(){
    console.log('Wuff');
}

Man.prototype.getInfo = function(method, url){
    var xhr = new XMLHttpRequest();

    xhr.open(method, url);
    xhr.send(null);

    xhr.onreadystatechange = function() {           
        console.log('Ajax response: ' + xhr.readyState);            
    };
}

var woman = new Woman();

console.log('woman instance of Woman ' + (woman instanceof Woman));
console.log('woman instance of Human ' + (woman instanceof Human));

woman.talk();

var man = new Man();

console.log('man instance of Man ' + (man instanceof Man));
console.log('man instance of Human ' + (man instanceof Human));

man.talk();

womans = [];
for(var i = 0; i < 1000; i++) {

    womans[i] = new Woman();        
}

document.body.onclick = function(){     
    var randNr = Math.floor((Math.random()*1000)+1);
    womans[randNr].talk();
    console.log('Random Woman: ' + randNr);
 }
 document.body.ondblclick = function(){
          Man.prototype.getInfo();
    }
share|improve this question

migrated from stackoverflow.com Feb 3 '14 at 22:25

This question came from our site for professional and enthusiast programmers.

    
I think this fits better at Code Review –  Bergi Jan 29 '14 at 22:01
    
ok thanks @Bergi, didn't knew about Code Review, i'll try it there to. Still any opinion is this a good start? –  Mr. Sam Jan 29 '14 at 22:06
    
We can move the question, you don't have to copy it. –  Bergi Jan 29 '14 at 22:07
    
Oh, greet, please do that, then –  Mr. Sam Jan 29 '14 at 22:07
    
@Bergi will i know when its moved or something? –  Mr. Sam Jan 29 '14 at 22:11

1 Answer 1

up vote 3 down vote accepted

Some opinions:

function Woman(){
    Human.call(this);
}
Woman.prototype = new Human();

Don't use new for prototype (even when it doesn't matter here as long as Human is empty, but the Huma.call(this) suggests otherwise). Check Correct javascript inheritance

// Woman subClass
function Man(){

Apparently not.

    var _foo = 10;

It's a local-scoped ("private") variable anyway, you don't need to prefix it with an underscore. Notice that it is only accessible from functions declared within the constructor, of which you don't have any.

    return Human.call(this);

No reason to return anything here. It might even be wrong it Human did return an object.

Man.prototype.getInfo = function(method, url){
    […]
}

This doesn't seem to have anything to do with instances of Man, so I wonder why it is a method? But that's probably just the design of your test case.

console.log('man private variable _foo: ' + (man._foo));

Not sure what you did expect, but yes, since _foo has been a variable and not a property of the object so this is undefined.

document.body.onclick = function(){     
    […]

…is missing a closing brace.

var randNr = Math.floor((Math.random()*1000)+1);
womans[randNr].talk();

No. That randNr will be in the range 1-1000 (both inclusive), while your array indices are 0-999. With a small chance, this will throw an exception. Remove the +1.

console.log('Random Woman: ' + randNr);

…and add it here if you want that output one-based.

Man.prototype.getInfo();

That confirms what I said above - you don't use an instance for that method. You should hardly ever call methods on the prototype object - man.getInfo() would be better. But if you really intended this to be a static function, you might write Man.getInfo = function() {…}; and then call Man.getInfo();.

share|improve this answer
    
I edited the code, please could you take a second and check if now the code is suffice for the test above. i would highly appreciate it. –  Mr. Sam Jan 30 '14 at 8:19
1  
Uh, actually I had only looked at the code since you can check whether it meets your expectatations by simply executing it :-) I'd say it does what is described, the only term that striked me was "private property" as such by definition don't exist :-) –  Bergi Jan 30 '14 at 13:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.