Weird issue with casting substring to integer

Question

I'm getting a weird issue with substringing. Apparently the string I get can't be cast into an Int32 for some odd reason. The error message I get when I try doing that is "input string is not in correct format". Because of this, I can't insert these values into the Database either.

Here's the code...

string width = GetMetadata(filename, 162); //returns "1280 pixels"
string height = GetMetadata(filename, 164); //returns "700 pixels"
width = width.Substring(0, width.IndexOf(' ')); //returns "1280"
height = height.Substring(0, height.IndexOf(' ')); //returns "700"

//test: "System.Convert.ToInt32(width)" will fail, giving error "input string was not in correct format"
//doing the above on "width" yields the same result

//fails, giving error "no such column: 1280" (underlying database is sqlite)            
Database.NonQuery("INSERT INTO image VALUES (" + fileid + ", " + width + ", " + height + ")");

You've put a breakpoint in there and absolutely verified that the values returned by `GetMetadata()` are what you expect? No extraneous spaces? No tabs or other incorrect whitespace? Because with the values "1280 pixels" and "700 pixels", that code works fine. — Carson63000, Jun 08 '11 at 06:42
@Carson : Yes, the values in the comments are exactly what I'm getting. I'll double check and post back if this is not the case. — rafale, Jun 08 '11 at 06:45
First of all - better to use parameters when working with database. What is the current culture on development machine? — vityanya, Jun 08 '11 at 06:49
It works fine when you assign width="1280 pixels". I agree with Carson63000 - double check the result of GetMetadata. — Renatas M., Jun 08 '11 at 06:50
Setting the starting index to 1 seems to have fixed the issue. There must have been whitespace or some other character there that I didn't see before. — rafale, Jun 08 '11 at 06:53

Jon Skeet · Accepted Answer · 2011-06-08T06:55:48.067

For all the normal reasons - primarily avoiding leaving data conversions to the database, and preventing SQL injection attacks - I would suggest that you perform the parsing to a number in C#, and then use a parameterized query to talk to SQLite.

In this case, that will make it a lot easier to debug - either .NET will fail to parse the string as well (in which case it's likely to be a problem with your data) or it will work, and you won't need to worry about what conversions database was performing.

EDIT: I've just seen your comment saying that Convert.ToInt32 fails as well. That's a pretty clear indication that it's the data which is causing a problem.

I'd expect your code to look something like this:

string widthText = GetMetadata(filename, 162);
string heightText = GetMetadata(filename, 164);
widthText = width.Substring(0, width.IndexOf(' ')).Trim();
heightText = height.Substring(0, height.IndexOf(' ')).Trim();

int width = int.Parse(widthText, CulutureInfo.InvariantCulture);
int height = int.Parse(widthText, CulutureInfo.InvariantCulture);

using (SQLiteCommand cmd = Database.CreateCommand())
{
    cmd.CommandText = "INSERT INTO image VALUES (?, ?, ?)";
    cmd.Parameters.Add(fileid);
    cmd.Parameters.Add(width);
    cmd.Parameters.Add(height);
    cmd.ExecuteNonQuery();
}

Note that the Trim call will remove any leading spaces, which it seems was the cause of the problem.

The problem has been solved, so I'm going off on a tangent here. Are there any advantages to using parameterized queries instead of normal queries besides protection against injection attacks? At the moment, I'm only using parameterized queries where user input needs to be inserted into the database. — rafale, Jun 08 '11 at 07:07
@rafale: It separates *code* (SQL) from *data* (values). It means you can see the data separately from the code really cleanly... as well as avoiding conversions taking place in the database when you've already got the data in a non-string form in your code. This is particularly important when transferring something like a date, where there are obviously several different formats for representing dates. — Jon Skeet, Jun 08 '11 at 07:10
I consider it bad practice to use non-parameterized queries when not dealing with user input. Then, every time you look at the query you have to ask yourself, "Hey, this is non-parameterized. Am I sure this isn't going to ever see user input, even in the future?" — Brian, Jun 08 '11 at 19:26

score 0 · Answer 2 · edited Jun 08 '11 at 08:44

0

There may be some stray whitespaces in the string variables width and height. Invoke Trim() method on the strings before casting them into integers:

width = width.Trim();
height = height.Trim();

Hope this helps. Let us know.

edited Jun 08 '11 at 08:44

DarthJDG

16,511
11
49
56

answered Jun 08 '11 at 06:56

sandyiscool

561
2
9

Weird issue with casting substring to integer

2 Answers2