3
\$\begingroup\$

I have a problem set which requires me to print all the multiples of a number in given range in reverse order.

So far I have tried this but not sure it is optimized or not!

#import <objc/objc.h>
#import <objc/Object.h>

@implementation TestObj

int main()
{
    int limit = 0;
    int divisor = 5;
    printf("enter a number : ");
    scanf("%d",&limit);

    printf("enter divisor : ");
    scanf("%d",&divisor);

    printf("Multiples of %d in reverse order till limit %d are : ",divisor, limit);
    for(int i = limit/divisor; i > 0 ; i--){
        printf("%d ",i*divisor);
    }

    return 0;
}

@end
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Apart from @implementation TestObj ... @end (which is completely unnecessary because you never create an instance of that class) this code is plain old C and no Objective-C at all. \$\endgroup\$
    – Martin R
    Commented Jul 21, 2015 at 11:42
  • 1
    \$\begingroup\$ This question would be drastically better asked as a C language question. Nothing about the main() function here is even remotely Objective-C. The stuff outside the main() function is entirely irrelevant to the main() function, isn't used in this snippet, isn't going to be reviewed by anyone. And on this site, the C tag certainly gets a lot more eyeballs than the Objective-C tag. Moreover, you're not importing Foundation, so even if there were some fancy OOP solution to this problem... a true Objective-C review would be tough... \$\endgroup\$
    – nhgrif
    Commented Jul 21, 2015 at 12:46

2 Answers 2

4
\$\begingroup\$

Multiplication is not a very cheap operation. Subtraction is much cheaper. It will be a better and cleaner algorithm to start from the highest multiple, and then keep subtracting until reaching 0:

int truncatedCount = limit / divisor;
int highestMultiple = truncatedCount * divisor;

for (int multiple = highestMultiple; multiple > 0; multiple -= divisor) {
    printf("%d ", multiple);
}

The improvements:

  • multiplication replaced by subtractions
  • more meaningful variable names (instead of i, for example)
  • more conventional spacing (around operators and around (...) and { ... }
\$\endgroup\$
4
  • \$\begingroup\$ I've always learned i is acceptable for an iterator, but only for an iterator. Did I learn wrong? \$\endgroup\$
    – Mast
    Commented Jul 21, 2015 at 12:11
  • \$\begingroup\$ Just because it's accepted doesn't mean it's preferred. \$\endgroup\$
    – nhgrif
    Commented Jul 21, 2015 at 12:12
  • 2
    \$\begingroup\$ Yes, i is often acceptable. But "acceptable" doesn't mean "great". I didn't criticize using i, but I saw an opportunity for improvement so I took it, and pointed it out as a recommendation. When there is an obvious better name than i, it's good to use it. Especially when i is not a regular run-of-the-mill counter from 0 to n but has more of a character as in this example. \$\endgroup\$
    – janos
    Commented Jul 21, 2015 at 13:03
  • \$\begingroup\$ This solution is good enough. I would like to take a test of time complexity and memory. \$\endgroup\$ Commented Jul 22, 2015 at 10:41
2
\$\begingroup\$

multiples of a number in given range in reverse

What I understood from that is, output the multiples of a given number for a given range in reverse. Say for example, if the number is 9 and the start range is 40 and end range is 65, the output will be: 63 54 45.

  • Why limit?

    int limit = 0;
    ..............
    printf("enter a number : ");
    scanf("%d",&limit);
    

    Instead it must be named as number.

  • Where is the given range? I don't see that in the code. Instead you are giving a divisor, which I am not sure is required or not.

    printf("enter divisor : ");
    scanf("%d",&divisor);
    
  • You must also optimize the iteration by iterating only on the multiples after you obtain the upper range multiple.

    You must have code similar to this, then:

    int number, startRange, endRange;
    // feed the above variables by the user.
    for(endRange -= endRange % number; endRange >= startRange; endRange-=number) {
      printf("%d ", endRange);
    }
    
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Please take more care of code formatting. \$\endgroup\$
    – Caridorc
    Commented Jul 21, 2015 at 11:52
  • \$\begingroup\$ int number would be like stating grass is green. Non-descriptive. \$\endgroup\$
    – Mast
    Commented Jul 21, 2015 at 22:24
  • \$\begingroup\$ Minimum range is the number itself. so starting from the number and go to the max limit range and print it reverse. This is the requirement. \$\endgroup\$ Commented Jul 28, 2015 at 13:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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