C# function best practice

Epicor ERP 10
1

I am working on an end activity screen and I am going to have some different buttons call end activity so I want to set up a function to call it and I would like to know what the best practice would be for setting the parameters.

I think I can just set the variables in before I call the function and not need any parameters, or I can set the variables and pass them trough the the function as parameters. I’m thinking that both ways accomplish what I want, but the parameters seem like I am just duplicating the number of variables that I need. Since getting the value from the grid is a pretty long chunk of code, I don’t think I want to put all of that into the parameter section of my function.

But is it better to have the parameters in the function? Is it basically just to make sure you don’t forget to set something in your code?

Any opinions?

2

Using parameters instead of class-level member variables makes your method more testable and portable/reusable. If that’s something you’re concerned about, method parameters might be more appealing.

2 Likes
3

Ok, so I am trying to set this up to use parameters. I have one set up for actually ending activity, which I think will work fine, mostly because it doesn’t need to return anything, it just does stuff.

The other one that I want to set up is a dynamic query. While it compiles, I am having problems because it does not returning anything. To start with, I’m just trying to run the function and return the results in a message box to see if it’s working.

Here is the function

	private void GetLaborDetails(string JN, string PN, string OC)
	{
		DynamicQueryAdapter gld = new DynamicQueryAdapter(oTrans);
		gld.BOConnect();
		QueryExecutionDataSet qeds = gld.GetQueryExecutionParametersByID("ClockOutBarcode");
		qeds.ExecutionParameter.Clear();
		qeds.ExecutionParameter.AddExecutionParameterRow("JobNum",JN,"nvarchar", false, Guid.NewGuid(),"A");
		qeds.ExecutionParameter.AddExecutionParameterRow("PartNum",PN,"nvarchar",false, Guid.NewGuid(),"A");
		qeds.ExecutionParameter.AddExecutionParameterRow("OpCode",OC,"nvarchar",false, Guid.NewGuid(),"A");
		gld.ExecuteByID("ClockOutBarcode");
		DataTable dt = gld.QueryResults.Tables["Results"];
	}

This is what I am trying to call with a button right now. The function seems to run fine, and the first message box shows with the pertinent info. However when I try to get the the datatable dt for the second message box, it pukes (Object reference not set to an instance of an object). I’m assuming it’s because that datatable is disposed of once it gets out of the function so I don’t really have dt to work with.

	private void EndWQtyButton_Click(object sender, System.EventArgs args)
	{
		// ** Place Event Handling Code Here **
		GetLaborDetails(JobBox.Text,PartNumBox.Text,OpBox.Text);
		MessageBox.Show("Test 0 button- JobNum:"+ JobBox.Text+" PartNum: "+PartNumBox.Text+" OpCode: "+ OpBox.Value );
		MessageBox.Show(dt.ToString());
	}

What’s confusing me is that this code. Will update the string and it’s available outside of the function. So I was assuming that the data table would be the same way. Apparently it isn’t though.

	private void addDataToMsg (string datastring)
	{
		ClickMsg += datastring ;
	}

I’m sure I am missing something fairly basic, so if anyone would be willing to educate me, that would be awesome!

4

In GetLaborDetails you are declaring dt as a new DataTable, not updating dt (which you must have as a class-level member variable, that you are accessing in the button click?).

If you want to update member variable dt instead, the code would look like this:

private void GetLaborDetails(string JN, string PN, string OC)
{
	DynamicQueryAdapter gld = new DynamicQueryAdapter(oTrans);
	gld.BOConnect();
	QueryExecutionDataSet qeds = gld.GetQueryExecutionParametersByID("ClockOutBarcode");
	qeds.ExecutionParameter.Clear();
	qeds.ExecutionParameter.AddExecutionParameterRow("JobNum",JN,"nvarchar", false, Guid.NewGuid(),"A");
	qeds.ExecutionParameter.AddExecutionParameterRow("PartNum",PN,"nvarchar",false, Guid.NewGuid(),"A");
	qeds.ExecutionParameter.AddExecutionParameterRow("OpCode",OC,"nvarchar",false, Guid.NewGuid(),"A");
	gld.ExecuteByID("ClockOutBarcode");
	dt = gld.QueryResults.Tables["Results"];
}

You could have GetLaborDetails return the datatable:

private DataTable GetLaborDetails(string JN, string PN, string OC)
{
	DynamicQueryAdapter gld = new DynamicQueryAdapter(oTrans);
	gld.BOConnect();
	QueryExecutionDataSet qeds = gld.GetQueryExecutionParametersByID("ClockOutBarcode");
	qeds.ExecutionParameter.Clear();
	qeds.ExecutionParameter.AddExecutionParameterRow("JobNum",JN,"nvarchar", false, Guid.NewGuid(),"A");
	qeds.ExecutionParameter.AddExecutionParameterRow("PartNum",PN,"nvarchar",false, Guid.NewGuid(),"A");
	qeds.ExecutionParameter.AddExecutionParameterRow("OpCode",OC,"nvarchar",false, Guid.NewGuid(),"A");
	gld.ExecuteByID("ClockOutBarcode");
	return gld.QueryResults.Tables["Results"];
}

And use it like this:

private void EndWQtyButton_Click(object sender, System.EventArgs args)
{
	// ** Place Event Handling Code Here **
	var dt = GetLaborDetails(JobBox.Text,PartNumBox.Text,OpBox.Text);
	MessageBox.Show("Test 0 button- JobNum:"+ JobBox.Text+" PartNum: "+PartNumBox.Text+" OpCode: "+ OpBox.Value );
	MessageBox.Show(dt.ToString());
}
1 Like
5

that snipped will not update the datastring outside of the scope of this method. You could have it return a string instead if that’s what it needs to do…

6

It is updating ClickMsg though, which is defined outside the scope of the method. I think that is where the confusion was.

7

Oh gotcha, I didn’t read very hard haha

8

Ok, that makes sense. I needed a reminder that declaring it again treats it like a new one.

Now it returns “Results” in the message box. I probably just need to do more works to get the data out of the datatable.

I’ll struggle for a while first then post any more questions I have.

9

I figured it out.I forgot to add my parameters set to my dynamic query adapter. Then I was sending in my parameters improperly (I needed opBox.Value.ToString(), since OpBox is referencing a combo box. Then I needed to loop through the Datarows, not just Rows.

So now this pops up a message box for each row with the job number in it, and it’s returning the right number of rows. So I now know how to get the information out of the query and the datatable that it returns.

	private void EndWQtyButton_Click(object sender, System.EventArgs args)
	{
		// ** Place Event Handling Code Here **
		
		dt = GetLaborDetails(JobBox.Text,PartNumBox.Value.ToString(),OpBox.Value.ToString());
		MessageBox.Show("Test qty button- JobNum:"+ JobBox.Text+" PartNum: "+PartNumBox.Text+" OpCode: "+ OpBox.Value.ToString() );
		foreach (DataRow r in dt.Rows)
		{
		MessageBox.Show(r["LaborDtl_JobNum"].ToString());
		}
	}

Thanks for your help guys!!

1 Like