2011-02-09 3 views
8
int uploadsID; 
int pageNumber; 
int x; 
int y; 
int w; 
int h; 

bool isValidUploadID = int.TryParse(context.Request.QueryString["uploadID"], out uploadsID); 
bool isValidPage = int.TryParse(context.Request.QueryString["page"], out pageNumber); 
bool isValidX = int.TryParse(context.Request.QueryString["x"], out x); 
bool isValidY = int.TryParse(context.Request.QueryString["y"], out y); 
bool isValidW = int.TryParse(context.Request.QueryString["w"], out w); 
bool isValidH = int.TryParse(context.Request.QueryString["h"], out h); 

if (isValidUploadID && isValidPage && isValidX && isValidY & isValidW & isValidH) 
{ 

Это обработчик ajax, проверяющий все пройденные параметры в порядке. Это считается плохим, и есть ли лучший способ написать это, или это не так важно?C# есть ли лучший способ написать это?

+1

Вы можете использовать объект, содержащий все свойства и один конструктор, для которых требуется NameValueCollection (Request.QueryString или просто Request) и подготовить объект, этот объект выставляет один метод: «IsValid» и проверяет, соответствуют ли все параметры текущему запросу. –

+1

Возможно, вы захотите опубликовать это на [codereview] (http://codereview.stackexchange.com/) – Benjol

ответ

7

Предполагая, что вы не собираетесь используйте переменную bool в другом месте, вы могут пишут, что, как:

int uploadsID, pageNumber, x, y, w, h; 
if (int.TryParse(context.Request.QueryString["uploadID"], out uploadsID) && 
    int.TryParse(context.Request.QueryString["page"], out pageNumber) && 
    int.TryParse(context.Request.QueryString["x"], out x) && 
    int.TryParse(context.Request.QueryString["y"], out y) && 
    int.TryParse(context.Request.QueryString["w"], out w) && 
    int.TryParse(context.Request.QueryString["h"], out h)) 
{ 
} 

Вы можете извлечь из int.TryParse(context.Request.QueryString[name], out variable в отдельный метод, оставив вас с чем-то вроде:

int uploadsID, pageNumber, x, y, w, h; 
if (TryParseContextInt32("uploadID", out uploadsID) && 
    TryParseContextInt32("page", out pageNumber) && 
    TryParseContextInt32("x", out x) && 
    TryParseContextInt32("y", out y) && 
    TryParseContextInt32("w", out w) && 
    TryParseContextInt32("h", out h)) 
{ 
} 

В качестве альтернативы, вы можете инкапсулировать все эти контекстные данные в новый тип с помощью метода TryParse, так что вам придется что-то вроде:

PageDetails details; 
if (PageDetails.TryParse(context.Request.QueryString)) 
{ 
    // Now access details.Page, details.UploadID etc 
} 

это, очевидно, больше работы, но я думаю, что это сделало бы более чистый код.

+0

Это хорошо спасибо :) –

2

Одна вещь, которую вы можете сделать, это заменить это:

int uploadsID; 
int pageNumber; 
int x; 
int y; 
int w; 
int h; 

С этим

int uploadsID, pageNumber, x, y, w, h; 
+0

Downvote в порядке, но причина будет оценена. Вы, очевидно, чувствуете, что первый код имеет превосходную читаемость для моего измененного кода? –

6

Да, начать факторинга ваши int.TryParse(etc.) в отдельную функцию.

(возможно, под влиянием сверх F #)

//return a tuple (valid, value) from querystring of context, indexed with key 
private Tuple<bool, int> TryGet(HttpContext context, string key) 
{ 
    int val = 0; 
    bool ok = int.TryParse(context.request.QueryString[key], out val); 
    return Tuple.New(ok, val); 
} 

Тогда:

var UploadId = TryGet(context, "uploadID"); 
//... 
if(UploadId.Item1 && etc..) 
{ 
    //do something with UploadId.Item2; 

Чтобы сделать вещи немного яснее, вы могли бы

private class ValidValue 
{ 
    public bool Valid { get; private set; } 
    public int Value { get; private set; } 
    public ValidValue(bool valid, int value) 
    { 
     Valid = valid; 
     Value = value; 
    } 
    //etc., but this seems a bit too much like hard work, and you don't get 
    // equality for free as you would with Tuple, (if you need it) 
+0

Больше похоже на отдельный класс, который проверяет его собственные свойства. – Turrau

+1

Не возражаете ли вы расширить это, пожалуйста? –

3

я бы, вероятно, пойти на форматирование, как этот


int uploadsID, pageNumber, x, y, h; 

if (Int32.TryParse(context.Request.QueryString["uploadID"], out uploadsID) 
    && Int32.TryParse(context.Request.QueryString["page"], out pageNumber) 
    && Int32.TryParse(context.Request.QueryString["x"], out x) 
    && Int32.TryParse(context.Request.QueryString["y"], out y) 
    && Int32.TryParse(context.Request.QueryString["w"], out w) 
    && Int32.TryParse(context.Request.QueryString["h"], out h)) 
{ 
    ... 
} 

, но я не вижу ничего плохого с вашим подходом.

1
try 
{ 
    // use Parse instead of TryParse 

    // all are valid, proceed 
} 
catch 
{ 
    // at least one is not valid 
} 
+0

Как правило, это плохая идея включать исключения как часть нормального потока программы, и в этом случае неправильный ввод пользователя, очевидно, является нормальной частью потока программы и должен * не * быть обрабатываются исключениями. Не очень важно, чтобы пользователь вводил что-то неправильно. –

+0

Это просто плохо. Не только вы используете исключения для потока управления, но и ущемляете производительность. – Stilgar

+0

Похоже, это хорошее решение для меня, если инструкция catch возвращает код ошибки или повторно бросает. @Stilgar: Вы действительно заботитесь о производительности в случаях ошибок? – adrianm

0

Вы можете написать помощник, который избавляется от уродливого out мимолетной стиле TryParse, таких как:

public delegate bool TryParser<T>(string text, out T result) where T : struct; 

public static T? TryParse<T>(string text, TryParser<T> tryParser) 
          where T : struct 
{ 
    // null checks here. 
    T result; 
    return tryParser(text, out result) ? result : new T?(); 
} 

А потом (если вы заинтересованы только в действительности):

bool isValid = new [] { "uploadID" , "page", "x", "y", "w", "h" } 
       .Select(q => context.Request.QueryString[q]) 
       .All(t => TryParse<int>(t, int.TryParse).HasValue); 

Если вам нужны отдельные значения:

var numsByKey = new [] { "uploadID" , "page", "x", "y", "w", "h" } 
       .ToDictionary(q => q, 
          q => TryParse<int>(context.Request.QueryString[q], 
               int.TryParse)); 

bool isValid = numsByKey.Values.All(n => n.HasValue); 

Это сохраняет почти ту же информацию, что и раньше, за исключением того, что мелкозернистая информация нуждается в поиске, а не в доступе к локальной переменной.

Смежные вопросы