help me to improve this program please

Discussion in 'C++' started by rami-madini@hotmail.com, May 30, 2014.

  1. Guest

    given two square matrices A & B of size n. based on user selection,
    preform one of the following operations:

    >> addition, subtraction, or multiplication of A & B.
    >> Transpose of A & B .
    >> Inverse of A or B only when n=2


    * Note A={a b} inverse A^-1= (1/ad-be){d -b}
    {c d} {-c a}

    this is my project for the c++ class I have read all of the instructions that i had received in my last

    this is my program help me pleas to improve it and thank you all


    #include<iostream>
    using namespace std;

    void main()
    {
    float A[10][10], B[10][10], C[10][10],h; int i, j, k, m, n, x, y, Select, c1;



    cout<<"Welcome to the matrix calculator :) \n";
    again:
    cout<<endl<<endl<<endl;


    cout<<"Select One Operation : \n"
    "(1)Addition, Subtraction, or Multiplication of A & B\n "
    "(2)Transpose of A or B\n"
    "(3)Inverse of A or B only when n= 2\n";


    cin>>Select;

    if(Select==1)
    {
    cout<<"choose: (1) For Addition.\t(2) For Subtraction.\t(3) For Multiplication.\n";
    cin>>c1;

    if(c1==1)
    {
    cout<<"Enter matrix dimensions:\n";
    cin>>m>>n;


    cout<<"\nEnter Matrix A values:\n";
    for(i=0; i<m; i++)
    {
    for(j=0; j<n; j++)
    {
    cin>>A[j];
    }
    }

    cout<<"\nEnter Matrix B values:\n";
    for(i=0; i<m; i++)
    {
    for(j=0; j<n; j++)
    {
    cin>>B[j];
    }
    }

    cout<<"\nSum of Matrix A and B:";
    for(i=0; i<m; i++)
    {
    cout<<"\n";
    for(j=0; j<n; j++)
    {
    C[j]=A[j]+B[j];
    cout<<C[j]<<"\t";
    }
    }
    }



    else if(c1==2)
    {
    cout<<"Enter matrix dimensions:\n";
    cin>>m>>n;


    cout<<"\nEnter Matrix A values:\n";
    for(i=0; i<m; i++)
    {
    for(j=0; j<n; j++)
    {
    cin>>A[j];
    }
    }

    cout<<"\nEnter Matrix B values:\n";
    for(i=0; i<m; i++)
    {
    for(j=0; j<n; j++)
    {
    cin>>B[j];
    }
    }

    cout<<"\nSubtraction of Matrix A and B:";
    for(i=0; i<m; i++)
    {
    cout<<"\n";
    for(j=0; j<n; j++)
    {
    C[j]=A[j]-B[j];
    cout<<C[j]<<"\t";
    }
    }
    }



    else if(c1==3)
    {
    cout<<"Enter Matrix A dimensions:\n";
    cin>>m>>n;

    cout<<"\nEnter Matrix A values:\n";
    for( i = 0 ; i < m ; i++)
    {
    for( j = 0 ; j < n ; j++)
    {
    cin>>A[j];
    C[j]=0;
    }
    }

    cout<<"Enter Matrix B dimensions:\n";
    cin>>x>>y;

    cout<<"\nEnter Matrix B values:\n";
    for( i = 0 ; i < x ; i++)
    {
    for( j = 0 ; j < y ; j++)
    {
    cin>>B[j];
    }
    }


    for( i = 0 ; i < m ; i++)
    {
    for( j = 0 ; j < y ; j++)
    {
    for( k = 0 ;k < n ; k++)
    {
    C[j] += A[k]*B[k][j];

    }
    }
    }

    cout<<"\nThe Multiplication of Matrix A and B:";
    for( i = 0 ; i < m ; i++)
    {
    cout<<"\n";
    for( j = 0 ; j < y ; j++)
    {
    cout<<C[j]<<"\t";
    }
    }
    }
    }





    else if(Select==2)
    {
    cout<<"Enter Matrix dimensions:\n";
    cin>>m>>n;
    cout<<"\nEnter Matrix Values:\n";

    for(i=0; i<m; i++)
    {
    for(j=0; j<n; j++)
    {
    cin>>A[j];
    B[j]=A[i][j];
    }
    }

    cout<<"\nThe transpose of the Matrix :";

    for(i=0; i<n; i++)
    {
    cout<<"\n";
    for(j=0; j<m; j++)
    {
    cout<<B[i][j]<<"\t";
    }
    }

    }

    else if(Select==3)

    {
    cout<<"Enter Matrix dimensions:\n";
    cin>>m>>n;
    if(n==2)
    {
    cout<<"\nEnter Matrix Values:\n";

    for(i=0; i<m; i++)
    {
    for(j=0; j<n; j++)
    {

    cin>>A[i][j];

    }
    }

    cout<<"\nThe Inverse of the Matrix :\n";

    h=(1/(((A[0][0])*(A[1][1]))-((A[0][1])*(A[1][0]))));
    cout<<(A[1][1]*h)<<"\t"<<(-A[0][1]*h)<<"\n"<<(-A[1][0]*h)<<"\t"<<(A[0][0]*h);
    }

    else
    cout<<"Invaled Input! Please read the statment again carefully this time :)";
    }
    else
    cout<<"Invaled Input! Please read the statment again carefully this time :)";

    goto again;
    cout<<endl;
    system("pause");

    }


    and thank you :)[/i][/i][/i]
     
    , May 30, 2014
    #1
    1. Advertisements

  2. There are several things you can and should do.

    One is realize that coding is one of the last steps in developing
    a program. Long before you code the first line, you should "design"
    your solution to the problem. This will help you put things in proper
    order, allow you to recognize blocks of common processing which should
    be implemented as separate functions, and identify other processing
    which could benefit from being in separate functions (to make the code
    easier to read, to simplify debugging, etc).

    Another, with a surprisingly high benefit for such a simple
    change, is to learn to indent consistently. The body of functions,
    the members of structures, the range of if and for statements, should
    be indented a visually discernible, but not excessive, amount. Most
    have found 3-5 space optimum.

    On Fri, 30 May 2014 07:25:49 -0700 (PDT),
    wrote:

    >given two square matrices A & B of size n. based on user selection,
    >preform one of the following operations:
    >
    >>> addition, subtraction, or multiplication of A & B.
    >>> Transpose of A & B .
    >>> Inverse of A or B only when n=2

    >
    >* Note A={a b} inverse A^-1= (1/ad-be){d -b}
    > {c d} {-c a}
    >
    > this is my project for the c++ class I have read all of the instructions that i had received in my last
    >
    >this is my program help me pleas to improve it and thank you all
    >
    >
    >#include<iostream>
    >using namespace std;
    >
    >void main()


    In a hosted system, main always returns int. ALWAYS!

    >{
    > float A[10][10], B[10][10], C[10][10],h; int i, j, k, m, n, x, y, Select, c1;


    While float is obviously acceptable, double is intended to be the
    natural floating point type for the system. There is almost no
    benefit to float unless you are really short of memory.

    Place only one declaration or statement on a line. Furthermore, it
    will simplify debugging if you place each object in its own
    declaration.

    The use of capital letters for simple local variables serves mostly to
    complicate typing.

    > cout<<"Welcome to the matrix calculator :) \n";


    Invest in horizontal white space. It is cost free and makes your code
    much more readable.

    > again:


    Labels and goto statements are never required and rarely appropriate.
    Try to use a loop for this.

    > cout<<endl<<endl<<endl;
    >
    >


    Excessive horizontal white space makes the code harder to follow and
    discourages others from helping.

    > cout<<"Select One Operation : \n"
    >"(1)Addition, Subtraction, or Multiplication of A & B\n "


    Never include tabs in a string literal. If you want a tab in the
    output (which is not the case here), use the \t escape sequence.

    > "(2)Transpose of A or B\n"
    > "(3)Inverse of A or B only when n= 2\n";


    Look at what your formatting does. First, the code is difficult to
    read. Second, the output to the user is not aligned:
    Select ...
    (1)...
    (2)...
    (3)...
    (4)...

    Consider coding it as:
    cout << "Select ...\n"
    " (1) ...\n"
    ...
    " (4) ...\n";

    > cin>>Select;
    >
    >if(Select==1)
    >{
    > cout<<"choose: (1) For Addition.\t(2) For Subtraction.\t(3) For Multiplication.\n";


    In this type of case, it is common to replace the \n with a few spaces
    so the user's input immediately follows the prompt.

    > cin>>c1;
    >
    > if(c1==1)
    > {
    >cout<<"Enter matrix dimensions:\n";
    > cin>>m>>n;
    >
    >
    >cout<<"\nEnter Matrix A values:\n";
    > for(i=0; i<m; i++)
    > {
    > for(j=0; j<n; j++)
    > {


    You might consider adding a prompt here
    cout << "Enter the value for row "
    << i << " column "
    << j << ": ";
    to make life easier for your user.

    > cin>>A[j];
    > }
    > }
    >
    >cout<<"\nEnter Matrix B values:\n";
    > for(i=0; i<m; i++)
    > {
    > for(j=0; j<n; j++)
    > {
    > cin>>B[j];
    > }
    > }
    >
    >cout<<"\nSum of Matrix A and B:";
    > for(i=0; i<m; i++)
    > {
    > cout<<"\n";
    > for(j=0; j<n; j++)
    > {
    > C[j]=A[j]+B[j];
    > cout<<C[j]<<"\t";


    Since you don't use C, why not just print the value of the sum? If
    you have learned about the iomanip header, you would be better off
    using a fixed width and left justification instead of tabs. With
    tabs, your data can look like (view with fixed font)
    1 2 3
    11 22 33
    With iomanip, it could look like
    1 2 3
    11 22 33

    > }
    > }
    > }


    > else if(c1==2)
    > {


    This entire block of code except for the subtraction is identical to
    the c1==1 case.

    >cout<<"Enter matrix dimensions:\n";
    > cin>>m>>n;
    >
    >
    >cout<<"\nEnter Matrix A values:\n";
    > for(i=0; i<m; i++)
    > {
    > for(j=0; j<n; j++)
    > {
    > cin>>A[j];
    > }
    > }
    >
    >cout<<"\nEnter Matrix B values:\n";
    > for(i=0; i<m; i++)
    > {
    > for(j=0; j<n; j++)
    > {
    > cin>>B[j];
    > }
    > }
    >
    >cout<<"\nSubtraction of Matrix A and B:";
    > for(i=0; i<m; i++)
    > {
    > cout<<"\n";
    > for(j=0; j<n; j++)
    > {
    > C[j]=A[j]-B[j];
    > cout<<C[j]<<"\t";
    > }
    > }
    > }


    > else if(c1==3)
    > {
    >cout<<"Enter Matrix A dimensions:\n";
    > cin>>m>>n;


    Before accepting values for A, you should get the dimensions for B and
    insure they are compatible with multiplication.

    >cout<<"\nEnter Matrix A values:\n";
    >for( i = 0 ; i < m ; i++)
    > {
    > for( j = 0 ; j < n ; j++)
    > {
    > cin>>A[j];
    > C[j]=0;
    > }
    > }
    >
    >cout<<"Enter Matrix B dimensions:\n";
    > cin>>x>>y;
    >
    >cout<<"\nEnter Matrix B values:\n";
    > for( i = 0 ; i < x ; i++)
    > {
    > for( j = 0 ; j < y ; j++)
    > {
    > cin>>B[j];
    > }
    > }
    >
    >
    > for( i = 0 ; i < m ; i++)
    > {
    > for( j = 0 ; j < y ; j++)
    > {
    > for( k = 0 ;k < n ; k++)
    > {
    > C[j] += A[k]*B[k][j];


    If you don't check the dimensions, you don't know if A[k] or
    B[k][j] even exist.

    > }
    > }
    > }
    >
    >cout<<"\nThe Multiplication of Matrix A and B:";
    > for( i = 0 ; i < m ; i++)
    > {
    > cout<<"\n";
    > for( j = 0 ; j < y ; j++)
    > {
    > cout<<C[j]<<"\t";
    > }
    > }
    >}
    > }
    >
    >
    >
    >
    >
    >else if(Select==2)
    >{
    > cout<<"Enter Matrix dimensions:\n";
    > cin>>m>>n;
    > cout<<"\nEnter Matrix Values:\n";
    >
    > for(i=0; i<m; i++)
    > {
    > for(j=0; j<n; j++)
    > {
    > cin>>A[i][j];
    > B[j][i]=A[i][j];
    > }
    > }
    >
    > cout<<"\nThe transpose of the Matrix :";
    >
    > for(i=0; i<n; i++)
    > {
    > cout<<"\n";
    > for(j=0; j<m; j++)
    > {
    > cout<<B[i][j]<<"\t";
    > }
    > }
    >
    >}
    >
    >else if(Select==3)
    >
    >{
    >cout<<"Enter Matrix dimensions:\n";
    > cin>>m>>n;[/i][/i][/i][/i]
    [i][i][i]

    Why are you asking for dimensions when the value for each has to be 2?
    [color=blue]
    > if(n==2)[/color]

    What does your code do if m is not 2?
    [color=blue]
    > {
    >cout<<"\nEnter Matrix Values:\n";
    >
    > for(i=0; i<m; i++)
    > {
    > for(j=0; j<n; j++)
    > {
    >
    > cin>>A[i][j];
    >
    > }
    > }
    >
    >cout<<"\nThe Inverse of the Matrix :\n";
    >
    > h=(1/(((A[0][0])*(A[1][1]))-((A[0][1])*(A[1][0]))));[/i][/color][i]

    You have way to many parentheses.
    h = 1 / ( A[0][0]*A[1][1] - A[0][1]*A[1][0] );
    [color=blue]
    > cout<<(A[1][1]*h)<<"\t"<<(-A[0][1]*h)<<"\n"<<(-A[1][0]*h)<<"\t"<<(A[0][0]*h);[/color]

    None of these parentheses serve any purpose.
    [color=blue]
    > }
    >
    > else
    > cout<<"Invaled Input! Please read the statment again carefully this time :)";[/color]

    An error message should tell the user where he made the mistake. Here
    n was not 2.
    [color=blue]
    >}
    >else
    > cout<<"Invaled Input! Please read the statment again carefully this time :)";[/color]

    Here, Select was not 1, 2, or 3. But 4 is a valid input which you do
    not accept.
    [color=blue]
    >
    > goto again;[/color]

    When posting code, convert your tabs to spaces since not everyone has
    the same setup you do.
    [color=blue]
    > cout<<endl;
    > system("pause");
    >
    >}
    >
    >
    >and thank you :)[/color]

    --
    Remove del for email[/i][/i][/i][/i]
     
    Barry Schwarz, May 31, 2014
    #2
    1. Advertisements

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.
Similar Threads
  1. Replies:
    4
    Views:
    951
    Chris Uppal
    May 5, 2005
  2. KK
    Replies:
    2
    Views:
    1,211
    Big Brian
    Oct 14, 2003
  3. MuZZy
    Replies:
    7
    Views:
    2,219
    Mike Hewson
    Jan 7, 2005
  4. goosen_cug
    Replies:
    9
    Views:
    587
    Dan Bloomquist
    Nov 10, 2006
  5. DG
    Replies:
    4
    Views:
    540
  6. Replies:
    14
    Views:
    715
  7. Esmail Bonakdarian

    1st program -- how would you improve this?

    Esmail Bonakdarian, Oct 5, 2006, in forum: Ruby
    Replies:
    9
    Views:
    263
    Esmail Bonakdarian
    Oct 6, 2006
  8. Tom Ewall

    Looking to improve program

    Tom Ewall, Nov 10, 2004, in forum: Perl Misc
    Replies:
    5
    Views:
    221
    John W. Krahn
    Nov 12, 2004
Loading...