1

I'm getting an error when I try to call the following procedure:

ApplyVoucherNumberToBillingCharges(practiceID,
                                  Convert.ToInt32(rdoVoucherNumberAppliesTo.SelectedValue), 
                                  VoucherNumber, 
                                  VoucherNumberID,
                                  Convert.ToInt32(hdRoundingRecordId.Value),
                                  ChargeID,
                                  hdGenerateVoucherNumberChargeList.Value,
                                  Convert.ToDateTime(txtFromDate.Text),
                                  Convert.ToDateTime(txtToDate.Text),
                                  Convert.ToDateTime(txtPostingDate.Text),
                                  Convert.ToInt32(ddlVisitType.SelectedValue),
                                  SelectVisitProvider.SelectedID,
                                  SelectVisitLocation.SelectedID,
                                  chkOverwriteVoucherNumber.Checked);

I'm getting the following error:

input string was not in a correct format c#

I know this has something to do with the convert class because I was getting the same error when calling a different method previously. The difference is there was only one parameters using the Convert class then and there are several now.

The error message isn't as descriptive as I would like and doesn't give many clues as to where in particular this is happening.

Besides commenting out and hard coding a value for each of the parameters that do this and seeing which one is causing the issue, is there a better way to identify what the issue is?

I would really like to know because my project takes several minutes to build sometimes (I know, it's ridiculous, I've tried everything to fix this but nothing works) and so debugging this little issue could take forever.

Thanks for any help you can provide.

maccettura
  • 10,514
  • 3
  • 28
  • 35
W.Harr
  • 303
  • 1
  • 4
  • 15
  • If you use `Convert.ToDateTime()` it will throw an exception if it cannot successfully convert. You should instead use `DateTime.TryParse()`. Of course this should be done _before_ you call this method so you can handle bad input before you call something that requires _good_ input – maccettura Apr 24 '18 at 20:46
  • The above comment is correct, but please post WHERE the error occurs, and the code that's throwing it... – Trey Apr 24 '18 at 20:47
  • Besides my above comment you are doing **zero** error checking elsewhere. What happens if `hdGenerateVoucherNumberChargeList` is null before you check `.Value`? Also, your method has like 100 parameters. This is typically a sign that you should be using an object to pass information. – maccettura Apr 24 '18 at 20:50
  • Maybe post the stack trace you get, there may be some indication as to which of the many parameters it is failing on. My gut feeling says the same as macettura that it is likely the DateTime conversion, it's not happy with the format even blank/null. – aggaton Apr 24 '18 at 20:51
  • Put a breakpoint on that line and hover over each of the values to see what may be causing the issue. – Sorceri Apr 24 '18 at 20:51
  • If you pass in a class you can have the setter/getter check for correct format and log any discrepancies. Though it is generally best to let it be up to the user of a class/function check validity rather than the class/function itself. – aggaton Apr 24 '18 at 21:05

1 Answers1

5

The snippet you've posted is the coding equivalent of a run-on sentence. You're performing far too many operations in what amounts to a single line of code. This makes your code difficult to read, and - as you've discovered - difficult to debug.

As a starting point, assign the results of your Convert calls to variables, then pass these into the method:

var convertedVoucherNumberAppliesTo = Convert.ToInt32(rdoVoucherNumberAppliesTo.SelectedValue);
//other conversions here
ApplyVoucherNumberToBillingCharges(practiceId, convertedVoucherNumberAppliesTo, \\ pass in other variables here...);

This way you will quickly be able to identify which conversion is failing as each potential error will be on a specific line.

As for your project taking minutes to build, depending on the project size, this could be normal. For example, I'm currently writing this answer waiting for a 20 minute build & test session on our CI server to complete - build takes up half of that time.

pixelbadger
  • 1,556
  • 9
  • 24
  • I think this answer is incomplete. Why not mention `TryParse()` as an alternative? Why not mention the 10,000 parameters for _one_ function? – maccettura Apr 24 '18 at 20:56
  • @maccettura I understand your suggestion about TryParse() but what difference would it make if I assigned all of the variables to an object and then passed the one object that contained them as attributes? – W.Harr Apr 24 '18 at 21:01
  • @W.Harr It would make [your code cleaner](https://stackoverflow.com/a/175035/2457029), it would make it easier to modify (modifying the class automatically modifies any method that accepts an instance of that class). With this many parameters it screams refactor – maccettura Apr 24 '18 at 21:07
  • @maccettura That's a good point about easily being able to modify a method just by changing the class. I will give your reference a read. Thanks for you help – W.Harr Apr 24 '18 at 21:16