Skip to main content
Code Review

Return to Answer

edited body
Source Link
Jerry Coffin
Jerry Coffin
  • 33.6k
  • 4
  • 75
  • 143

This declarationsdeclaration is bad practice: See: https://stackoverflow.com/q/1452721/14065 second answer is the best.

LetsLet's have a Look:

One variable per line. Only declare variables at the point you need them. This is not C98C89. You don't need to declare all the variables at the top. It gives you no speed benifitbenefit to declare them all at the top. It gives you no speed benifitbenefit to declare variables that are only 1 character lone give them a name so we can read the code.

This declarations is bad practice: See: https://stackoverflow.com/q/1452721/14065 second answer is the best.

Lets have a Look:

One variable per line. Only declare variables at the point you need them. This is not C98. You don't need to declare all the variables at the top. It gives you no speed benifit to declare them all at the top. It gives you no speed benifit to declare variables that are only 1 character lone give them a name so we can read the code.

This declaration is bad practice: See: https://stackoverflow.com/q/1452721/14065 second answer is the best.

Let's have a Look:

One variable per line. Only declare variables at the point you need them. This is not C89. You don't need to declare all the variables at the top. It gives you no speed benefit to declare them all at the top. It gives you no speed benefit to declare variables that are only 1 character lone give them a name so we can read the code.

added 663 characters in body
Source Link
Loki Astari
Loki Astari
  • 96.6k
  • 5
  • 125
  • 338

You have a couple of compiler warnings:

24: error: ISO C++ forbids initialization of member ‘pos’
24: error: making ‘pos’ static
24: error: ISO C++ forbids in-class initialization of non-const static member ‘pos’
24: error: local class ‘struct main()::minim’ shall not have static data member ‘long int main()::minim::pos’
25: error: ISO C++ forbids initialization of member ‘value’
25: error: making ‘value’ static
25: error: ISO C++ forbids in-class initialization of non-const static member ‘value’
25: error: local class ‘struct main()::minim’ shall not have static data member ‘long int main()::minim::value’

Not a valid header:

Not a valid header:

You have a couple of compiler warnings:

24: error: ISO C++ forbids initialization of member ‘pos’
24: error: making ‘pos’ static
24: error: ISO C++ forbids in-class initialization of non-const static member ‘pos’
24: error: local class ‘struct main()::minim’ shall not have static data member ‘long int main()::minim::pos’
25: error: ISO C++ forbids initialization of member ‘value’
25: error: making ‘value’ static
25: error: ISO C++ forbids in-class initialization of non-const static member ‘value’
25: error: local class ‘struct main()::minim’ shall not have static data member ‘long int main()::minim::value’

Not a valid header:

Source Link
Loki Astari
Loki Astari
  • 96.6k
  • 5
  • 125
  • 338

Not a valid header:

#include <bits/stdc++.h>

This declarations is bad practice: See: https://stackoverflow.com/q/1452721/14065 second answer is the best.

using namespace std;

One declaration per line please.

    long long stock, cost;

You are making copies of the parameter here.

bool compare (sorted_food lhs, sorted_food rhs)

Pass by const reference to avoid a copy.
If you had written your types above properly you have spotted that it was more expensive to copy than passing a reference.

This looks completely garbage:

    return (lhs.cost < rhs.cost && lhs.cost && rhs.cost);

Lets have a Look:

    A Cost  B Cost    A < B    B < A
       5      6         T        F           OK
       5      0         F        F           Hmmmm So they are equal?
       0      6         F        F           Hmmmm
       0      0         F        F           Hmmmm

What on earth!!!

    long long i,n,m,cost,d,j,k=1;

One variable per line. Only declare variables at the point you need them. This is not C98. You don't need to declare all the variables at the top. It gives you no speed benifit to declare them all at the top. It gives you no speed benifit to declare variables that are only 1 character lone give them a name so we can read the code.

If you can read the code you have a better chance of spotting better algorithms.

This is a bit eccentric way of writing a loop!

    for (i=1; i<=m; i++)

    // Normally written like this:
    for (int i=0; i < m; ++i)

Yep, as expected the rest is unreadable so I can't suggest a better algorithm.