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'm trying to read data from an array. I manage to get the information I want but my code is extremely inefficiently written.

My array looks like this:

var arTab:Array = new Array();
arTab[0] = new Array(2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012);
var promilleTab:Array = new Array();
promilleTab[0] = new Array(9631, 8593, 8363, 8128, 8514, 8534, 8560, 8146, 8241, 8019,         8759);
var forerkortTab:Array = new Array();
forerkortTab[0] = new Array(13481, 12785, 12585, 12492, 13470, 14181, 14622, 14082, 14287, 13640, 14180);
var hastighetTab:Array = new Array();
hastighetTab[0] = new Array(9863, 12217, 14920, 14929, 15425, 18010, 15909, 14197, 13276, 11158, 12264);
var trafikklovenTab:Array = new Array();
trafikklovenTab[0] = new Array(18862, 19196, 20101, 20026, 21381, 21845, 20005, 19446, 20900, 19346, 20265);

I am using combo boxes to pick a year from the "arTab" array and another to pick the felony done.(the 3 other arrays)

What I am doing right now looks something like this:

function skrivUt(Event:MouseEvent)
{
    if(bruddListe == 1)
    {
        if(arListe == 1)
        {
            txtSvar.text = "I " + arTab[0][0] + " ble det anmeldt " + promilleTab[0][0] + " for promillekjøring";
        }
        if(arListe == 2)
        {
            txtSvar.text = "I " + arTab[0][1] + " ble det anmeldt " + promilleTab[0][1] + " for promillekjøring";
        } 
        if(arListe == 3)
        {
            txtSvar.text = "I " + arTab[0][2] + " ble det anmeldt " + promilleTab[0][2] + " for promillekjøring";
        }
    {
{

This is just a small example but the list goes on because I have to do 11 results for all the 4 combo choices.

Is there any way I can write this in a shorter and more efficient way?

share|improve this question

migrated from stackoverflow.com May 23 '14 at 18:06

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

2 Answers 2

Sure,

you are basically copy pasting to keep accessing arTab and promilleTab, you might instead just derive the indexes in arTab from bruddListe and arListe, like this:

function skrivUt(Event:MouseEvent)
{
  txtSvar.text = "I " + arTab[bruddListe-1][arListe-1] + " ble det anmeldt " + promilleTab[bruddListe-1][arListe-1] + " for promillekjøring";

etc. etc.

or you could keep things a bit more readable, by splitting it up, I dont know if this is correct ActionScript, but conceptually you should grok this:

function skrivUt(Event:MouseEvent)
{
  var ar:String =  arTab[bruddListe-1][arListe-1];
  var promille:String = promilleTab[bruddListe-1][arListe-1];
  txtSvar.text = "I " + ar + " ble det anmeldt " + promille + " for promillekjøring";

Furthermore, some other observations :

  • Try to keep your variables and function Names in English, it's the right thing to do
  • it seems af if bruddListe and arListe are global variables, you should avoid those
share|improve this answer

Use a better data structure. Instead of using several arrays, you can create only one of Data.

public class Data {
            public var year: int;
            public var promille: int;
            public var driversLicense: int;
            public var speed: int;
            public var trafikkloven: int;
            public function Data(year, promille, driversLicense, speed, traffikloven: int) {
                this.year = year;
                this.promille = promille;
                ...
            }
            public function getText(): String {
                 return "I " + year + " ble det anmeldt " + promille + " for promillekjøring";
            } 
}

However, Array's are not typesafe. It is better to use Vector (assuming you're using ActionScript 3).

var datas: Vector.<Data> = new Vector.<Data>();
datas.push(new Data(2002, 9631, 13481, 9863, 18862));
txtSvar.text = datas.get(0).getText();

However, as this would make it hard to remember the ordering of the parameters, I suggest that you look at the Builder pattern

share|improve this answer

Your Answer

 
discard

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