Thread: Is it good?

Code:

#include <string>
#include <vector>
using std::string;
using std::vector;
using std::cout;
using std::endl;

#define MAX_ITEMS 100
#define MAX_INV 30

class Items
{
		int ID;
		int mainClass;
		int subClass;
		int grade;
		int quality;
		int degree;
		int bonding;
		int price;
		int stackSize;
		int primDmg[3];
		int sconDmg[3];
		int resist[3];
		int bnsOpt[10];
	    string name;
	    string desc;
		double attSpeed;
		double defSpeed;

public:

	Items(){
		ID = NULL; 
		mainClass = NULL;
		grade = NULL;
		quality = NULL;
		degree = NULL;
		bonding = NULL;
		price = NULL;
		attSpeed = NULL;
		defSpeed = NULL;}

	void ItemsInfo (int A, int B, int C, int D, int E, int F, int G, int H, int I, int J, int K,
		int L, int M, int N, int O, int P, int Q, int R, int S, int T, int U, int V, int W, int X,
		int Y, int Z, int AA, int AB, string AC, string AD, double AE, double AF)
	    {
		    ID = A;
		    mainClass = B;
		    subClass = C;
		    grade = D;
		    quality = E;
		    degree = F;
		    bonding = G;
		    price = H;
		    stackSize = I;
		    primDmg[0] = J;  // first element directly adds to the needed stat
		    primDmg[1] = K;
		    primDmg[2] = L;
		    sconDmg[0] = M;  // first element directly adds to the needed stat
		    sconDmg[1] = N;
		    sconDmg[2] = O;
		    resist[0] = P;
		    resist[1] = Q;
	    	resist[2] = E;
	    	bnsOpt[0] = S;       //each array here is directly added to the player stats
	    	bnsOpt[1] = T;
	    	bnsOpt[2] = S;
	    	bnsOpt[3] = U;
	    	bnsOpt[4] = V;
	    	bnsOpt[5] = W;
	    	bnsOpt[6] = Y;
	    	bnsOpt[7] = Z;
	    	bnsOpt[8] = AA;
	    	bnsOpt[9] = AB;
	        name = AC;
	        desc = AD;
		    attSpeed = AE;
		    defSpeed = AF;};

		double Quality()
		{
			if (quality==0) return 0.7;
			if (quality==1) return 1.0;
			if (quality==2) return 1.3;
			if (quality==3) return 1.7;
			if (quality==4) return 2.1;
			if (quality==5) return 2.6;
			if (quality==6) return 3.1;}

		double Grade()
		{
			if (grade==0) return 1.0;
			if (grade==1) return 1.1;
			if (grade==2) return 1.2;
			if (grade==3) return 1.4;
			if (grade==4) return 1.6;
			if (grade==5) return 2.0;
			if (grade==6) return 2.4;
		    if (grade==7) return 3.2;}

		double Degree()
		{
			if (degree==0) return 1.0;
			if (degree==1) return 1.1;
			if (degree==2) return 1.2;
			if (degree==3) return 1.4;
			if (degree==4) return 1.6;
			if (degree==5) return 1.9;
			if (degree==6) return 2.2;
		    if (degree==7) return 2.6;
			if (degree==8) return 3.0;
			if (degree==9) return 3.5;
			if (degree==10) return 4.0;
			if (degree==11) return 4.6;
			if (degree==12) return 5.2;
			if (degree==13) return 5.9;
			if (degree==14) return 6.6;}

		int ArmRes(){return (resist[0]*(int)Quality())*(int)Grade();}
		int EleRes(){return (resist[1]*(int)Quality())*(int)Grade();}
		int MagRes(){return (resist[2]*(int)Quality())*(int)Grade();}

		int MinDmg(){return (primDmg[1]*(int)Quality())*(int)Grade();}
		int MaxDmg(){return (primDmg[2]*(int)Quality())*(int)Grade();}
		int Min2Dmg(){return (primDmg[1]*(int)Quality())*(int)Grade();}
		int Max2Dmg(){return (primDmg[2]*(int)Quality())*(int)Grade();}

		void showOpts()
		{
			string type;
			for (int iii=0; iii!=10; iii++)
				if (bnsOpt[iii]!=0)
				{
					if (iii==0)
						type = "DEX";
					else if (iii==1)
						type = "STR";
					else continue;
					
					cout << "Adds" << bnsOpt[iii] << type << endl;} }

		void descrWin()
		{
			/*showName();
			showInf1();
			showDesc();
			showStts();*/
			showOpts();
			/*showInf2();*/}


}itemsDB[MAX_ITEMS], charInv[MAX_INV], charEqup[10];

Well the good think it works fine, but im still wondering if i can improve it somehow, any suggestion? but don't mention the argu's.

Originally Posted by matsp

Definitely, having 20+ arguments to a function is BAD. Not naming them something sensible makes it DOUBLY bad.

Since some of those are arrays, why not pass in an array/vector instead?

--
Mats

well i did something like that in the past but i didn't like the fact that i had to use multiple functions for one object, and is it the only thing i should improve?

Originally Posted by anon

Code:

ID = NULL; 
mainClass = NULL;
//...

NULL is for pointers (although in C++ it is defined as 0).



My compiler says that parameters R and X are unused.

Code:

		double Grade()
		{
			if (grade==0) return 1.0;
			if (grade==1) return 1.1;
			if (grade==2) return 1.2;
			if (grade==3) return 1.4;
			if (grade==4) return 1.6;
			if (grade==5) return 2.0;
			if (grade==6) return 2.4;
		    if (grade==7) return 3.2;}

This and similar functions don't handle the case where grade is not one of the values. (assert / throw an exception / return some default value like 0.0)

Code:

int ArmRes(){return (resist[0]*(int)Quality())*(int)Grade();}

In this and similar functions. Why make Quality and Grade return a double in the first place, if you are always truncating it to int before using? Perhaps the intention was to make the calculation with doubles, and only cast the final result?

well if it is the same as 0 its ok

oh, then maybe its just some little typo i had

the value is not user-input, its a fixed value, so theres no need to do the assert IMO

the double value id for some other purpose, in that case i need a whole number

Originally Posted by anon

All these things produce compiler warnings, so they are not completely OK.



It takes more typing and it doesn't mean the same thing semantically.



The value comes from the outside and is not checked anywhere. Asserts like this help you catch accidental errors when your mind wonders.

well i think i will change it to 0 then (tho i always loved the NULL keyword)

hmm i guess ill use it then, it makes more sense, well thanks for the help, I really appreciate it.

Originally Posted by matsp

Going completely off-topic here, but the US TV program(me) Mythbusters showed that you can. I think Lion poo was one of the better ones to work on... [the principle is one of a japanese art of polishing balls of mud/clay or some such].

The fact that a turd MAY be polished to a gloss doesn't fix the problems with too many arguments tho'.

--
Mats

Interesting, LOL!!!
I didn't see that episode!