getting lowest and greatet common factors

Hi,

I think that this is an errata, but I want to check this with you guyz.

I have a method called HasComFactor() method to get the lowest and greatest common factors if they exist.
The original code is in the below.  
However, I have a problem with
int max = x < y ? x : y;

I think that it should be
int max = x < y ? y : x;

For 3 and 21,
it cannot find the lcf and gcf with 'int max = x < y ? x : y;', so I think that this is an error.

Am I right?

class MyClass{
    public bool HasComFactor(int x, int y, out int lcf, out int gcf)
    {
        int max = x < y ? x : y;
        bool first = true;

        lcf = 1;
        gcf = 1;

        // Find lcf and gcf
        for (int i = 2; i < max/2 + 1; i++)
        {
            if(((y%i)==0) & ((x%i)==0))
            {
                if(first)
                {
                    lcf = i;
                    first = false;
                }
                gcf = i;
            }
        }// for
        if(lcf !=1)
            return true;
        else
            return false;
    }
}// MyClass

class demo{    
    static void Main()
    {
        MyClass ob = new MyClass();

        int lcf, gcf;
        if(ob.HasComFactor(3, 21, out lcf, out gcf))
        {
            Console.WriteLine("LCF of 3 and 21 is " + lcf);
            Console.WriteLine("GCF of 3 and 21 is " + gcf);
        }
        else
            Console.WriteLine("No common factor for 231, 105");    
        Console.Read();
    }
}
IzzyTwinklyAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

pivarCommented:
Hi,

Yes, to get max you should use  x < y ? y : x

Try this:

	class MyClass {
		public bool HasComFactor(int x, int y, out int lcf, out int gcf)
		{
			int max = x < y ? y : x;
			bool first = true;

			lcf = 0;
			gcf = 0;

			// Find lcf and gcf
			for (int i = 1; i < max/2 + 1; i++)
			{
				if(((y%i)==0) & ((x%i)==0))
				{
					if(first)
					{
						lcf = i;
						first = false;
					}
					gcf = i;
				}
			}// for
			if(lcf != 0)
				return true;
			else
				return false;
		}
	}// MyClass

Open in new window


/peter
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
ToddBeaulieuCommented:
You can also just use the framework math functions! It certainly makes for cleaner code (easier to understand) and you can use the more advanced functions when need be, as well.

int max = System.Math.Max(x, yield);
0
TommySzalapskiCommented:
The code has an error. If x and y are 25 and 50, then technically, the GCF is 25.
So you need to test for this.
Also, the original was right codewise and was just poorly named. There is no need to test any numbers greater than min/2 (other than min itself). So the variable should have been named min, not max.

My fixes are below
class MyClass {
  public bool HasComFactor(int x, int y, out int lcf, out int gcf)
  {
    int max = x > y ? x : y;
    int min = x < y ? x : y;
    bool foundLCF = false;
    bool foundGCF = false;

    lcf = 0;
    gcf = 0;
    
    if(max%min == 0)
    {
      foundGCF = true;
      gcf = min;
    }

    // Find lcf and gcf
    for (int i = 2; i < max/2 + 1 && (!foundGCF || !foundLCF); i++)
    {
      if(((y%i)==0) & ((x%i)==0))
      {
        if(!foundLCF)
        {
          lcf = i;
          foundLCF = true;
        }
        if(!foundGCF)
          gcf = i;
      }
    }// for
    if(lcf != 0)
      return true;
    else
      return false;
  }
}// MyClass 

Open in new window


Also, since you are not considering 1 to count as the LCF, pivar's code needs to start at 2 as well. But 1 is always the LCF, so perhaps the description needs to be modified.
0
TommySzalapskiCommented:
Oops. On line 19 replace max with min.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
C#

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.