0
\$\begingroup\$

I am making a fractions calculator that can calculate fractions, whole numbers and mixed fractions.

So there is a String fraction, which is the input.

Also, the user will enter the fraction in a specified format which is:

For normal fraction, "a/b" (a and b are integers)

For whole numbers, "c" (c is an integer)

For mixed fraction, "c a/b" (c is integer, a/b is the fraction separated by a space)

I want the program to check and find out which type of fraction the user has entered and then proceed. I'm using If-Else If-Else for this.

Here's a code snip:

fun fractionalize(fraction: String): List<Int>{
        //Finds type of number (input) and converts to List<Int> with [numerator, denominator, whole number]
        var result: List<Int> = listOf(0,1,0)
        var numerator: Int = 0
        var denominator:Int = 1
        var whole_number:Int = 0
        try {
            if ((fraction.contains('/') && fraction.indexOf('/') == fraction.lastIndexOf('/')) && !fraction.contains(" ")) {
                //input is a regular fraction
                var fraction_string = fraction.split("/")
                numerator = fraction_string[0].toInt()
                denominator = fraction_string[1].toInt()
                result = listOf(numerator, denominator, 0)
            } else if (fraction.isDigitsOnly()) {
                //input is a whole number
                result = listOf(fraction.toInt(), 1, 0)
            } else if ((fraction.contains('/') && fraction.indexOf('/') == fraction.lastIndexOf('/')) && (fraction.contains(" ") && fraction.indexOf(' ') == fraction.lastIndexOf(' '))) {
                //input is mixed fraction
                var wholeNumber_split = fraction.split(" ")
                whole_number = wholeNumber_split[0].toInt()
                numerator = wholeNumber_split[1].split("/")[0].toInt()
                numerator += whole_number * denominator
                denominator = wholeNumber_split[1].split("/")[1].toInt()
                result = listOf(numerator, denominator, 0)
            }
            else {
                Toast.makeText(applicationContext,
                "The number you entered is not in a valid format.",
                Toast.LENGTH_LONG).show()
            }
            //simplifying input
            if (result[2] != 0){
                numerator += whole_number * denominator
                result = listOf(numerator, denominator, 0)
            }
            return result
        }
        catch (e: Exception){
            Toast.makeText(applicationContext,
                "The number you entered is not in a valid format.",
                Toast.LENGTH_LONG).show()
            return result
        }
    }

Is this the best way I can do that or is there more cleaner and efficient code I can make (personally I think this is too messy and can be made more efficient)? Please guide me.

\$\endgroup\$
  • 2
    \$\begingroup\$ A regular expression is the first thing that comes to my mind when I have to check if input matches a certain pattern. It would allow you to check full validity of the input, not just if it contains a '/' character. E.g. 'a/3' would be rejected immediately. \$\endgroup\$ – TorbenPutkonen Jun 30 at 4:47
  • \$\begingroup\$ @TorbenPutkonen So what should I keep as parameters? \$\endgroup\$ – DeathVenom Jun 30 at 4:57
  • 4
    \$\begingroup\$ Please adjust the title of your "question" to confirm to this site's standards. \$\endgroup\$ – Roland Illig Jun 30 at 6:53
0
\$\begingroup\$

Your code has several areas where it can be improved.

The first is that you should separate the parsing code from the platform code (Toast). Kotlin has a great testing framework, in which it is easy to write "parsing "1 2/3" should return listOf(1, 2, 3)". This testing code works best if you run it directly on your developing machine, and without any Toast or Activity.

The next thing is that you should not use List<Int> in places where you originally wanted to say Fraction or maybe, to be more specific, MixedFraction. The point is that listOf(1, 2, 3, 4, 5) is a valid list but not a valid fraction. By using a specifically constrained type you force the Kotlin compiler to check your code for you.

class Fraction(
    val int: Int,
    val num: Int,
    val den: Int
)

To make your parser extensible to expressions that are more complicated, you must not use contains. Instead, the best-practice way is to split the input into tokens first (often using regular expressions) and then to combine these tokens to expression trees (using a parser that only looks at the very next token to decide what to do next). The details of all of this are covered in any good book on compiler construction.

In your case, the tokens would be:

  • unsigned integer
  • fraction slash
  • unary minus sign

Note that there is no "space" token, as that is usually left out when defining an expression language.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I am fairly new to android development, so I don't understand a lot of things you told me.....Making a Fraction class is a great idea. I don't get making the parser thing. Please help me. Also, I'll edit the code to include the whole function. \$\endgroup\$ – DeathVenom Jun 30 at 5:46
  • 1
    \$\begingroup\$ Just google for "kotlin testing", and you should find a tutorial for that topic. Invest some time in this topic since testing is a fundamental requirement to writing reliable and robust code. \$\endgroup\$ – Roland Illig Jun 30 at 6:19
  • 3
    \$\begingroup\$ Please, don't answer incomplete questions. Now there are 2 options: either you remove the remark about the code being incomplete or we rollback the added code. I'll leave it up to you which is preferred. Thank you. \$\endgroup\$ – Mast Jun 30 at 6:23
  • \$\begingroup\$ @Mast I think removing the remark would be better. \$\endgroup\$ – DeathVenom Jun 30 at 6:31
  • 1
    \$\begingroup\$ @DeathVenom Next time, please make sure the first revision of your question is complete. We don't deal well with snippets, as indicated in the help center. \$\endgroup\$ – Mast Jun 30 at 6:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.