return a sorted array in increasing order containing all the factorial numbers <= n

[modern grail]

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
public class Solution {
    
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        long n = sc.nextLong();
        System.out.println(factorialNumbers(n));
    }
    public static List<Long> factorialNumbers(long n) {
        
        ArrayList<Long> finallist = new ArrayList<>();
        long k = 1;
        while(k<=n) {
            long a = factorial(k);
             if (a < n){
                 finallist.add(a);
             }
             k++;
        }
        return finallist;
    }
                             
    private static long factorial(long n){    
        if(n==0){

            return 1;
        }
        long result = n * factorial(n-1);
        return result;
    }
}```

so am getting the output nearly correct..there is some gibberish printed at the end..can someone tell me why

[granite sundial]

Detected code, here are some useful tools:

Formatted code

package allfactorial;

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class Solution {
  public static void main(String[] args) {
    Scanner sc = new Scanner(System.in);
    long n = sc.nextLong();
    System.out.println(factorialNumbers(n));
  }
  public static List<Long> factorialNumbers(long n) {
    ArrayList<Long> finallist = new ArrayList<>();
    long k = 1;
    while (k <= n) {
      long a = factorial(k);
      if (a < n) {
        finallist.add(a);
      }
      k++;
    }
    return finallist;
  }
  private static long factorial(long n) {
    if (n == 0) {
      return 1;
    }
    long result = n * factorial(n - 1);
    return result;
  }
}

<@&987246399047479336> please have a look, thanks.

[granite sundial]

While you are waiting for getting help, here are some tips to improve your experience:

Code is much easier to read if posted with syntax highlighting and proper formatting.

If nobody is calling back, that usually means that your question was not well asked and hence nobody feels confident enough answering. Try to use your time to elaborate, provide details, context, more code, examples and maybe some screenshots. With enough info, someone knows the answer for sure.

Don't forget to close your thread using the command </help-thread close:1027500463647621170> when your question has been answered, thanks.

Here is an AI assisted attempt to answer your question 🤖. Maybe it helps! In any case, a human is on the way 👍. To continue talking to the AI, you can use </chatgpt:1108714622413963314>.

public class FactorialNumbers {
public static void main(String[] args) {
Scanner scanner = new Scanner(System.in);
System.out.print("Enter a number: ");
int n = scanner.nextInt();

    List<Integer> factorialNumbers = getFactorialNumbers(n);
    
    System.out.println("Factorial numbers <= " + n + ": " + factorialNumbers);
}

public static List<Integer> getFactorialNumbers(int n) {
    List<Integer> factorialNumbers = new ArrayList<>();
    
    for (int i = 1; i <= n; i++) {
        int factorial = 1;
        for (int j = 1; j <= i; j++) {
            factorial *= j;
        }
        
        if (factorial <= n) {
            factorialNumbers.add(factorial);
        } else {
            break;
        }
    }
    
    return factorialNumbers;
}

}

[modern grail]

exceeds time limit here

nvm I just added another if loop
if(a>n) {
break;
}

[lilac moon]

u are calculating the factorials over and over again

[frozen jacinth]

The exceeding time limit is because your code is not optimized. Think about it.
To calculate factorial(k), your method will be computing factorial(k-1), factorial(k-2), etc...
And when you go for factorial(k+1), you're doing all that work once again.

You're basically going all the way up to n, but for each value k, you're digging back all the way to 0.
Can you try to rewrite this code to take advantage of earlier calculations ?

[modern grail]

I just added another if loop to break out of it the factorial value crossed n
all test cases passed

[frozen jacinth]

Good for you 🙂 Though what Nem and I tried to explain still applies, if you are interested.

[modern grail]

didnt I optimize my code by starting with calculating factorial of 1, moving upwards and checking if factorial of next number was crossing value of n or not and then breaking out if it was

I am completely new to recursion so i am not sure if u meant to optimize in a different way

[lilac moon]

u can optimize by using dynamic programming/memoization

[modern grail]

this is my 6th day of learning java

and the lesson I followed asked to use recursion

dp is very advanced for me rn

[lilac moon]

if u call
factorial(2) then factorial(3) ur calculating factorial(2) twice and so on

[modern grail]

oh

ahh

[lilac moon]

so u could memoize the result of factorial(2) e.g. in an array and use it to calculate factorial(3)

[modern grail]

can I store it as a long value?

[lilac moon]

sure

[frozen jacinth]

You're already storing it in your resulting list

[modern grail]

so I understand what u guys r saying

but am not able to code it

so should I just simply erase the factorial function?

my code doesnt look too clean so when I try making changes I mess up

[lilac moon]

okay so after u added the result of ur factorial call to the list, how can u calculate the next factorial number with that result?

[modern grail]

ya am wondering the same

[lilac moon]

so lets try it with factorial(3), which is 6, how can u calculate factorial(4) by using that six?

[modern grail]

just multiply by k

which is 4

[lilac moon]

okay do you know how to index arrays?

[modern grail]

thats what u meant right

[lilac moon]

yes

[modern grail]

but is this recursive?

[lilac moon]

u could drop the a and instead use finallist.get to find the last factorial result

[modern grail]

ya but how will ik the index of the last element

[lilac moon]

how many elements did you add in the k'th iteration?

[modern grail]

not sure

btw I have to do this ques using recursion I guess I forgot to mention that

😢

[lilac moon]

think about it. every iteration u add one element

[lilac moon]

we can do it recursively on the next step

[modern grail]

so finallist.get(k) ?

or k-1

[lilac moon]

yes

but u need to add the base case first so 1

[modern grail]

getting slightly confused now

[lilac moon]

its awkward because the solution asks for the factorial numbers from 1..n but if u want to "align" the list index with (k-1) u need to start from factorial(0)

so u need to do

list.add(1); // factorial(0)
for(k = 1;k<=n;k++) {
  list.add(k * list.get(k-1)); 
}
list.remove(0) // drop factorial(0) from the result

[granite sundial]

Detected code, here are some useful tools:

[modern grail]

Um not recursive?

[lilac moon]

i need to leave soon, soo if u want to transform it into recursion try to implement:

static List<Long> factorials(long n) {
  var factorials = new ArrayList<Long>();
  factorials.add(1); // base case
  factorials = factorials(factorials, n, 1);
  factorials.remove(0);
  return factorials;
}

private static List<Long> factorials(List<Long> factorials, long n, long k) {
  // TODO implement recursively
}

[granite sundial]

Detected code, here are some useful tools:

[lilac moon]

but tbh ur probably supposed to calculate the factorial recursively and not use memoization/dp..

[modern grail]

I understand but this was given in the recursion series

[lilac moon]

its a bad excercise then because its highly ineffective to recalculate the factorial over and over again. unless they want to show the problem with that in the next step 😉

[modern grail]

Is there any other recursive way to do it

[lilac moon]

u could implement the loop here recursively

but otherwise not really

[modern grail]

Ah ok

Thx a lot

[lilac moon]

btw u could optimize further by using an array of size n instead of a list

[modern grail]

so the challenge required me to take a list

[lilac moon]

ok it doesnt matter anyway but keep in mind that List cant hold primitives while array can