[Javascript] Beginnersvraag mbt. opbouw code

[roughtodacore]

Hallo iedereen,

Ik ben sinds kort begonnen met programmeren in Javascript via een online training. Nu had ik een opdracht gekregen waarbij als je op een knop drukt, mijn functie uitgevoerd wordt.

Betreft volgende code:

JavaScript:

function dropoff()
{    
    // declare seat check
    var seatcheck = 0;
            
    // loop through all seats, increment when a seat is null
    for (seats in shuttle.seats)
    {
        if (shuttle.seats[seats] === null)
        {
            // increment counter
            seatcheck ++;
        }       
    }
    
    // return if seatcheck equals the number of possible seats
    if (seatcheck === shuttle.seats.length)
    {
        $("#announcements").html("The shuttle is empty! Please pick up some students first.");
        return;
    }
    
    // define count variable for dropped off users
    var count = 0;
    
    // loop through houses array
    for (var house in HOUSES)
    {   
        // assign d to distance between every destination house and shuttle
        var d = distance(HOUSES[house].lat, HOUSES[house].lng, shuttle.position.latitude, shuttle.position.longitude);
        
        // if house is in range
        if (d <= 30.0)
        {                                          
            // loop through passenger list aboard the shuttle
            for (var i in shuttle.seats)
            {                      
                // only taken spots in shuttle
                if (shuttle.seats[i] != null)
                {
                    // if a user's house is in range
                    if (shuttle.seats[i].house === house)
                    {            
                        // remove user from view table
                        for (var j in viewarray)
                        {
                            if (shuttle.seats[i] === viewarray[j])
                            {
                                viewarray[j] = null;
                            }
                        }
                        
                        // drop off user
                        shuttle.seats[i] = null;
                          
                        // increment count variable
                        count ++;
                        
                        // update chart
                        chart();                        
                    }
                }
            }  
        }
    }
    
    // inform user how many
    if (count == 1)
    {
        $("#announcements").html("dropped off " + count + " user.");
    }
    
    // inform user how many
    else if (count > 1)
    {
        $("#announcements").html("dropped off " + count + " users.");
    }    
    
    // if there isn't a house in range
    else if (d > 30.0 && count == 0)
    {
        $("#announcements").html("There is no house within range of the shuttle!");
    }     
}

Context:

Ik heb een busje, welke gespawned wordt binnen mijn Google Maps API. Met dit busje kan ik rijden in mijn 3D wereld. Ik heb een array met personen, deze worden random gedropped in mijn 3D wereld. Deze mensen kan ik oppikken met mijn busje. Daarna moeten ze bij hun bestemming afgezet worden. Deze functie verzorgt het afzetten van de personen die in mijn busje zitten.

Er zijn een paar voorwaarden waaraan voldaan moet worden (bijv. dat als je bij een bestemming in de buurt bent, deze binnen bereik van het busje is etc.). De code werkt prima, maar ben benieuwd of ik het niet allemaal wat te omslachtig gecode heb.

Als voorbeeld ben ik benieuwd of er een makkelijkere manier is om te kijken of mijn array vol is (zie begin van functie), nu heb ik het idee dat ik zeer omslachtig bezig ben.

Tevens heb ik een vraag over de opbouw van mijn laatste stuk code. Ik indent 5 keer voordat er pas echt code uitgevoerd gaat worden.

Pas aan het eind van de functie kijk ik adv. bepaalde waarden wat voor error (of message eigenlijk) ik moet uitspugen naar de gebruiker.

Concreet ben ik dus een n00b die graag van zijn eventuele gemaakte fouten wilt leren, en andere ervaren programmeurs evt. een leuke maandagochtend wilt geven (mocht mijn code wel heeeel slecht wezen).

Mijn dank is groot!

[NMe]

For ... in gebruik je in Javascript bij voorkeur niet om over arrays te loopen, of in elk geval niet zonder een hasOwnProperty-check. Verder: waarom zou je lege items in je array houden voor lege plekken? Waarom niet los opslaan hoeveel plekken je hebt en zolang je array een kortere length heeft dan dat heb je nog plekken vrij.

Overigens zijn je comments veelal zinloos. Comments zijn met name bedoeld om je gedachtenproces uit te leggen aan de lezer (of jezelf), niet om te zeggen wat een regel precies doet. Het commentaar op regels 3, 11, 16, 26, 35, 56, 60 en 73 is redelijk nutteloos. Het commentaar op regel 23 zou overbodig zijn als de variabele niet count maar droppedOffCount zou heten. Het commentaar op regel 29 is foutief omdat je juist de afstand assignt aan d en niet andersom.

[incaz]

Ik zou allereerst een onderscheid maken door een nieuw object te maken: passenger. En niet de shuttle maar de passenger heeft de eigenschappen zoals 'house' (of misschien 'address' of 'destination').

Vervolgens maak je bij je shuttle keurig een addPassenger en removePassenger, en een functie getEmptySeats, en handige properties om dropoff-distance en aantal seats te definieren.

Wees verder vooral ook niet bang om javascript inbuilt functies te gebruiken om iets in je arrays te vinden (bv indexOf)

[pingwings]

Het is ook altijd handig om je code te verdelen in meerdere functies, zodat je bepaalde controles of gedrag kunt hergebruiken.

[R4gnax]

Eens met incaz.
Modeleer wat meer je domein en dan wordt je code ineens een stuk vanzelfsprekender.

[roughtodacore]

Dank allen voor jullie antwoorden! Heb nog gegoogled adv. jullie adviezen en ben weer een heel stuk wijzer geworden . Vooral de HasOwnProperty functie doet mijn ogen openen.

Ik gebruik inderdaad bepaalde stukken code vaker, en heb deze inderdaad ook in functies gegoten.

Ik heb nog 1 vraag. Ik heb hier een stuk code welke dat zichzelf nog een keer herhaalt binnen een while-loop:

JavaScript:

for (var i = 0; i < PASSENGERS.length; i++)
{   
    // randomly select building to spawn passenger at
    var building = BUILDINGS[Math.floor(Math.random() * BUILDINGS.length)];
       
    // loop through all houses
    for (var house in HOUSES)
    {              
        // if house matches destination house passenger
        if (house == PASSENGERS[i].house)
        {
            // calculate distance between spot passenger will be dropped and destination
            var d = distance (building.lat, building.lng, HOUSES[house].lat, HOUSES[house].lng);
 
            // keep reassigning a new building until distance is more then 100.0
            while (d < 100.0)
            {
                building = BUILDINGS[Math.floor(Math.random() * BUILDINGS.length)];                 
                d = distance (building.lat, building.lng, HOUSES[house].lat, HOUSES[house].lng);
            }
        }               
    }
}

En ja, ik zie dat het zelfde commentaar van jullie van toepassing is op dit stukje code. Maar de code lijkt mij zo omslachtig, kan dit simpeler syntaxtueel gezien?

[ Voor 7% gewijzigd door roughtodacore op 25-08-2014 22:36 ]

[R4gnax]

Zo zou het nog iets beter kunnen met iets minder herhaling en minder runtime complexiteit.

Als je toch aan alle passagiers een start gebouw moet toe kennen en je al kunt gaan vergelijken of een huis in de array huizen gelijk is aan het huis toegekend aan de passagier, dan kun je net zo goed meteen het huis gebruiken wat toegekend is aan de passagier i.p.v. over die hele array huizen heen te gaan lopen.

JavaScript:

function assignStartingBuildings( passengers, buildings ) {
  var
    passenger,
    building,
    house,
    n,
    d;

  for ( n = passengers.length ; --n !== -1 ; ) {
    passenger = passengers[ n ];
    house = passenger.house;

    // Keeps randomly pick a building to spawn a passenger at until succesfully picking
    // a building that is more than the required distance away from the passenger's
    // destination (house).
    for ( d = 0 ; d < 100 ; ) {
      building = buildings[ Math.floor( Math.random() * buildings.length )];
      d = distance( building.lat, building.lng, house.lat, house.lng );
    }
    
    passenger.building = building;
  }
}

[roughtodacore]

R4gnax schreef op dinsdag 26 augustus 2014 @ 01:12:
Als je toch aan alle passagiers een start gebouw moet toe kennen en je al kunt gaan vergelijken of een huis in de array huizen gelijk is aan het huis toegekend aan de passagier, dan kun je net zo goed meteen het huis gebruiken wat toegekend is aan de passagier i.p.v. over die hele array huizen heen te gaan lopen.

Bedankt voor jouw antwoord! Leuk om zo met for-loopjes te spelen (de initializers), geeft een bult nieuwe mogelijkheden!

Wat je zegt klopt niet echt. Dit omdat de huizen waar passagiers gespawned worden niet in de houses array (dus de destination adressen) voorkomt, maar in de buildings array. Ik moet dus eerst loopen over de houses array, zien of de huis van de passagier overeenkomt met 1 van de huizen uit de array. Zo ja, dan pas kan ik berekenen wat de afstand is tussen dat huis en de building.

Ik snap jullie verbeterpunten helemaal! En heb mijn laatste stuk code al mooi op weten te poetsen Toch raar dat ik jullie verbeteringen begrijp, maar dit nog niet uit mij zelf toepas.

[incaz]

roughtodacore schreef op maandag 25 augustus 2014 @ 22:34:

Het schreeuwt in elk geval om 2 functies:

JavaScript:

function getRandomBuilding();
// en
Obj.getDistanceTo( otherObject );