1

I am working on a tax calculator and had the idea of putting the tax brackets into a multi-dimensional array. This is the first time I have tried such a thing and was wondering if this is the correct way of doing it?

private static double[][][] taxBrackets;

static {
    taxBrackets = new double[20][14][14];
/*Single*/                           /*Married*/                        /*Head of Household*/
    //TierOne                           //TierOne                           //TierOne
    taxBrackets[0][0][0] = 0;           taxBrackets[0][1][0] = 0;           taxBrackets[0][0][1] = 0;       //MIN
    taxBrackets[1][0][0] = 9075;        taxBrackets[0][2][0] = 18150;       taxBrackets[0][0][2] = 12950;   //MAX
    taxBrackets[2][0][0] = 0.10;                                                                         //Tax Rate
    //TierTwo
    taxBrackets[3][0][0] = 9076;        taxBrackets[0][3][0] = 18151;       taxBrackets[0][0][3] = 12951;   //MIN
    taxBrackets[4][0][0] = 36900;       taxBrackets[0][4][0] = 73800;       taxBrackets[0][0][4] = 49400;   //MAX
    taxBrackets[5][0][0] = 0.15;                                                                         //Tax Rate
    //TierThree
    taxBrackets[6][0][0] = 36901;       taxBrackets[0][5][0] = 73801;       taxBrackets[0][0][5] = 49401;   //MIN
    taxBrackets[7][0][0] = 89350;       taxBrackets[0][6][0] = 148850;      taxBrackets[0][0][6] = 127550;  //MAX
    taxBrackets[8][0][0] = 0.25;                                                                         //Tax Rate
    //TierFour
    taxBrackets[9][0][0] = 89351;       taxBrackets[0][7][0] = 148851;      taxBrackets[0][0][7] = 127551;  //MIN
    taxBrackets[10][0][0] = 186350;     taxBrackets[0][8][0] = 226850;      taxBrackets[0][0][8] = 206600;  //MAX
    taxBrackets[11][0][0] = 0.28;                                                                        //Tax Rate
    //TierFive
    taxBrackets[12][0][0] = 186351;     taxBrackets[0][9][0] = 226851;      taxBrackets[0][0][9] = 206601;  //MIN
    taxBrackets[13][0][0] = 405100;     taxBrackets[0][10][0] = 405100;     taxBrackets[0][0][10] = 405100; //MAX
    taxBrackets[14][0][0] = 0.33;                                                                        //Tax Rate
    //TierSix
    taxBrackets[15][0][0] = 405101;     taxBrackets[0][11][0] = 405101;     taxBrackets[0][0][11] = 405101; //MIN
    taxBrackets[16][0][0] = 406750;     taxBrackets[0][12][0] = 457600;     taxBrackets[0][0][12] = 432200; //MAX
    taxBrackets[17][0][0] = 0.35;                                                                        //Tax Rate
    //TierSeven
    taxBrackets[18][0][0] = 406751;      taxBrackets[0][13][0] = 457601;     taxBrackets[0][0][13] = 432201; //MIN
    taxBrackets[19][0][0] = 0.396;                                                                       //Tax Rate
}

The idea was to have Single filers in the left array, Married/Joint filers in the middle and Head of Household filers in the right array. Did I do this correctly or did I miss the point entirely?

UPDATE: 1/30/2017 After following Nhouser9's suggestions I ended up with this which is much more manageable.

private static class TaxBracket {

    final double minSalary;
    final double maxSalary;
    final double taxRate;

    TaxBracket(double minSalary, double maxSalary, double taxRate) {
        this.minSalary = minSalary;
        this.maxSalary = maxSalary;
        this.taxRate = taxRate;
    }
}
//This is the data structure which holds the values for each tier of each filing status
//Changing values in these arrays will affect the output of the entire program
private static TaxBracket[] singleFiler;
static {
    singleFiler = new TaxBracket[]
        {
            new TaxBracket(0, 9075, 0.10),      //Index 0 TierOne
            new TaxBracket(9076, 36900, 0.15),  //Index 1 TierTwo
            new TaxBracket(36901, 89350, 0.25), //Index 2 TierThree
            new TaxBracket(89351, 186350, 0.28),//Index 3 TierFour
            new TaxBracket(186351, 405100, 0.33),//Index 4 TierFive
            new TaxBracket(405101, 406750, 0.35),//Index 5 TierSix
            new TaxBracket(406751, Double.MAX_VALUE, 0.396)//Index 6 TierSeven
        };
}
private static TaxBracket[] jointFiler;
static {
    jointFiler = new TaxBracket[]
        {
            new TaxBracket(0, 18150, 0.10),      //Index 0 TierOne
            new TaxBracket(18151, 73800, 0.15),  //Index 1 TierTow
            new TaxBracket(73801, 148850, 0.25), //Index 2 TierThree
            new TaxBracket(148851, 226850, 0.28),//Index 3 TierFour
            new TaxBracket(226851, 405100, 0.33),//Index 4 TierFive
            new TaxBracket(405101, 457600, 0.35),//Index 5 TierSix
            new TaxBracket(457601, Double.MAX_VALUE, 0.396)//Index 6 TierSeven
        };
}
private static TaxBracket[] hohFiler;
static {
    hohFiler = new TaxBracket[]
        {
            new TaxBracket(0, 12950, 0.10),      //Index 0 TierOne
            new TaxBracket(12951, 49400, 0.15),  //Index 1 TierTow
            new TaxBracket(49401, 127550, 0.25), //Index 2 TierThree
            new TaxBracket(127551, 206600, 0.28),//Index 3 TierFour
            new TaxBracket(206601, 405100, 0.33),//Index 4 TierFive
            new TaxBracket(405101, 432200, 0.35),//Index 5 TierSix
            new TaxBracket(432201, Double.MAX_VALUE, 0.396)//Index 6 TierSeven
        };
}
Gabriel_W
  • 1,645
  • 5
  • 16
  • 26
  • 4
    This looks *extremely over-complicated and unreadable - why not define a simple method taking 2 or 3 parameters and outputting the correct tax rate? As I do not have much of an idea of taxes I do not know what parameters the method should take, but putting them in one data structure seems very odd and wrong. – luk2302 Jan 29 '17 at 19:29
  • For the record: fore working code, you could better turn to codereview.stackexchange.com – GhostCat Jan 29 '17 at 19:38
  • @GhostCat Thanks for the tip, I will check it out. – Gabriel_W Jan 29 '17 at 19:41

3 Answers3

5

This is one possible solution. If it works, it's fine.

Since Java is an Object Oriented language, It would be better to use Objects. For example, TaxBracket object like this:

public class TaxBracket {
    public static final int SINGLE = 1;
    public static final int MARRIED = 2;
    public static final int HEAD_OF_HOUSE = 3;

    public final int mStatus;
    public final int minSalary;
    public final int maxSalary;
    public final double rate;

    public TaxBracket(int mStatus, int minSalary, int maxSalary, double rate) {
        this.mStatus = mStatus;
        this.minSalary = minSalary;
        this.maxSalary = maxSalary;
        this.taxRate = taxRate;
    }
}

Then instead of multi-dimensional arrays you could declare a single array of these objects, initializing it like this:

taxBrackets[0] = new TaxBracket(TaxBracket.SINGLE, 0, 9075, .1);
nhouser9
  • 6,730
  • 3
  • 21
  • 42
  • @nhouser9 I like this, it makes way more sense than what I had. Thank you for the feedback. I will work on implementing this and let you know how it goes. – Gabriel_W Jan 29 '17 at 20:01
  • @Gabriel Happy to help! Let us know how it goes, and if it solves your problem remember to mark it as the accepted answer =] – nhouser9 Jan 29 '17 at 20:44
  • @nhouser9 got it working. I really appreciate your help. – Gabriel_W Jan 29 '17 at 22:00
3

No. It is not the best way to do that. I would implement three classes for Single, Married and HeadOfHousehold.

Each of this class would implement an interface, e.g. Married implements TaxTiers.

interface TaxTiers() {

  public int getMaxForTier(int tier) {
  }

  public int getMinForTier(int tier) {
  }

  public double getRateForTier(int tier) {
  }
}

In classes Single, Married and HeadOfHousehold you can have 3 arrays or Lists: tierMins, tierMaxs and tierRates. Then you just write:

public double getRateForTier(int tier) {
  return tierRates.get(tier);
}
Grzegorz Górkiewicz
  • 4,496
  • 4
  • 22
  • 38
  • I am doing this later in the code but I just wanted a good way to store the values and make it easy to update and maintain later down the road. – Gabriel_W Jan 29 '17 at 19:31
  • @Gabriel if you want to make it easy to update and maintain, do it the right way the first time around. That's how you get it to be maintainable, not by implementing a hack which you plan to fix later. That is how so so so so many maintainability problems come up. – nhouser9 Jan 29 '17 at 19:33
  • @nhouser9 I agree totally that's why I wanted to post this so I can learn the correct way. I am a student and quite new to Java. – Gabriel_W Jan 29 '17 at 19:35
  • Just for the record: the problem with such code is that you will start **duplicating** switch statements. And that is **bad**. Instead: you want to have a **factory** that returns instances of some interface; and the factory does the switch in ONE place. – GhostCat Jan 29 '17 at 19:38
  • @GhostCat, agree with duplication of switch statement. Any ideas? – Grzegorz Górkiewicz Jan 29 '17 at 19:39
  • Indeed. But do you see what can be done with my code to implement your idea? – Grzegorz Górkiewicz Jan 29 '17 at 19:42
  • See here for example: http://stackoverflow.com/questions/126409/ways-to-eliminate-switch-in-code ... the point is to figure if polymorphism could be used here. – GhostCat Jan 29 '17 at 19:47
  • Edit. This way I can avoid long switch statements... The only downside is that tier is not a detached entity. – Grzegorz Górkiewicz Jan 29 '17 at 20:11
1

I would prefer to create a class for the tax bracket contains the attributes Single, Married, HeadOfHousehold and all other attributes you have. And, initiate a single dimension array of instances of that class. Like the following:

class TaxBracket{
    double single;
    double married;
    double headOfHousehold;

    .... // any other attributes + getters and setters! 

}

In the main class, you define the array:

TaxBracket []taxBrackets = new TaxBracket[20];
Kh.Taheri
  • 946
  • 1
  • 10
  • 25