CODEWITHSUNDEEP
javaperformanceandroid-studioif-statementconditional-statements
Julius Saikauskas
Julius Saikauskas

Reputation: 69

Is there any code I could write that is more effective and clean rather than a bunch of else if statements?

I'm more of an intermediate android dev and I want to learn how to write more clean and effective code, and I've come to this problem on my app that requires to check a bunch of statements like if a variable is less than something or more for each individual rating.

private void rateStock(double peRatio) {
    //For Dividend Stocks
    if(currentDiv != 0 && fiveYearDiv != 0){
        double dividendDiff = currentDiv - fiveYearDiv;
        if (peRatio <= 20 && peRatio > 0  && dividendDiff >= 0.01 && payoutRatio <= 0.65) {
            int stockRatingDividend = 5;
            addItems(stockRatingDividend);
        } else if (peRatio > 20 && peRatio <= 25 && dividendDiff >= 0.005 && payoutRatio <= 0.75) {
            int stockRatingDividend = 4;
            addItems(stockRatingDividend);
        } else if (peRatio > 25 && peRatio <= 30 && dividendDiff >= 0.001 && payoutRatio <= 0.85) {
            int stockRatingDividend = 3;
            addItems(stockRatingDividend);
        } else if (peRatio > 30 && peRatio <= 35 && payoutRatio <= 0.95) {
            int stockRatingDividend = 3;
            addItems(stockRatingDividend);
        } else if( peRatio > 35 &&  peRatio <= 40 && currentDiv > fiveYearDiv && payoutRatio >= 100) {
            int stockRatingDividend = 2;
            addItems(stockRatingDividend);
        } else if( peRatio > 40 || peRatio < 0 && fiveYearDiv > currentDiv  && payoutRatio >= 100) {
            int stockRatingDividend = 1;
            addItems(stockRatingDividend);
        } else {
            int stockRatingDividend = 0;
            addItems(stockRatingDividend);
        }
    }
}

As you can see by the code above it's very messy and cluttered and doesn't even work correctly, because there are too many conditions it has to check and it usually doesn't fit into any else if statement and just returns 0

I know it might be a lot but can someone at least guide me to a post or something on how I could write more effective and clean code rather than a cluttered mess of else if statements for this block of code?

Upvotes: 1

Views: 104

Answers (3)

Julius Saikauskas
Julius Saikauskas

Reputation: 69

For any future wanderers, that have the same question, I have come up with a solution myself!

What I did was split it into different classes and in the first class ValueLists.java I created all the ranges and made a getter for the list.

public List<Range> getPeRatioRanges() {

        Range range20 = new Range(0, 20);
        Range range25 = new Range(21, 25);
        Range range30 = new Range(26, 30);
        Range range35 = new Range(31, 35);
        Range range40 = new Range(36, 40);

        List<Range> list = new LinkedList<>(Arrays.asList(range40, range35, range30, range25, range20)); // add in reverse order cuz index = rating

        return list;
    }

And in the other class CheckRatingOfStock.java I created a simple loop for iterating through the ranges and get the rating.

 List<Range> peRatioRanges = getPeRatioRanges();

            for (int i = 0; i < peRatioRanges.size(); i++) {
                Range range = peRatioRanges.get(i);
                int intPERatio = (int) rawPEratio;

                if (range.contains(intPERatio)) {
                    rating = i + 1;
                }
            }

Here the index of the range is the rating + 1, so if its 0-20 its the rating of 5, because the list is reversed. This way with 2 blocks of code I avoid a lot of back and forth and the clutter of mass else if statements, and also make it much more easily maintainable and cleaner, atleast in my opinion.

This could be implemented as a method:

static int getIndexOfRange(int intPERatio, List<Range> ranges)
{
    for (int i=0; i< ranges.size(); i++) 
       if (ranges.get(i).contains(intPERatio)) 
           return i+1;                         //if found, return immediatelly
              
    return -1;   //if it reaches this line, means the item was not found
 }               //return -1 as representation of this

Upvotes: 1

juwil
juwil

Reputation: 466

the possible values of your parameters are not fully represented by the choices in youre if/else branches. So the best way would be to implement a class which decides by itself, if it is responsible for the computation of your single resulting value stockRatingDividend. a sketch of this (with no getters, constructor and no handling, if one border for the min/max is not set) would be:

public class SRDResolver {
    public class SRDRolver {
        private int peRatioMin;
        private int peRatioMax;
        private double dividendDiffMin;
        private double dividendDiffMax;
        private double payoutRatioMin;
        private double payoutRatioMax;

        private double stockRatingDividend;

        public boolean hasSolution(int peRatio,double dividendDiv,double payoutRatio){
            return peRatio > peRatioMin && peRatio <= peRatioMax  && dividendDiv > dividendDiffMin && dividendDiv <= dividendDiffMax && payoutRatio > payoutRatioMin && payoutRatio <= payoutRatioMax;
        }
    }

    
}

then you put the intances in a list. You then get the correct instance from streaming that list with filter(i-> i.hasSolution(...).findFirst(), which gives you an Optional as a result. If it is present, you take it's stockRatingDividend,otherwise you use your fallback value of 0. That way you can easyily add new instances, if you need,without cluttering if/else. You might add some checks, when you put the instances into the list to ensure, that the number spaces span of the instances do not overlap.

Upvotes: 2

Gautham M
Gautham M

Reputation: 4935

The below two could be combined as it does the same thing.

else if (peRatio > 25 && peRatio <= 30 && dividendDiff >= 0.001 && payoutRatio <= 0.85) {
    int stockRatingDividend = 3;
    addItems(stockRatingDividend);
} else if (peRatio > 30 && peRatio <= 35 && payoutRatio <= 0.95) {
    int stockRatingDividend = 3;
    addItems(stockRatingDividend);
}

Also, I would recommend refactoring the if-else block to a new method which returns stockRatingDividend. This would keep the dividend computation logic separate from the other logic, which would actually make the code more understandable.

if(currentDiv != 0 && fiveYearDiv != 0){
    double dividendDiff = currentDiv - fiveYearDiv;
    addItems(getStockRatingDividend(/*arguments*/));
}

Upvotes: 0

Related Questions