2014-01-19 2 views
0

Я только начинаю программировать на C#, поэтому я новичок, я практиковал некоторые коды, и мне хотелось бы, чтобы ваше мнение о чем-то.Предложения по методу?

У меня есть направление полета для плоскости, например. «Лондон - Берлин» (направление). Я хочу создать метод, который вернет первый и последний согласный начальной точки самолета (Лондон), и первый и последний согласный в пункте назначения самолета (Берлин). Я написал это, так что я хотел бы знать, если это нормально, или если у вас есть какие-то предложения:

public class Flight 
{ 
    private string direction; 

    public Flight(string direction) 
    { 
     this.direction = direction; 
    } 

    public string Consonants() 
    { 
     string starting = direction.Split('-')[0]; 
     string destination = direction.Split('-')[1]; 

     string startConsonants = starting.ToUpper().Replace("A", "").Replace("E", "").Replace("I", "").Replace("O", "").Replace("U", ""); 
     string destConsonants = destination.ToUpper().Replace("A", "").Replace("E", "").Replace("I", "").Replace("O", "").Replace("U", ""); 

     return string.Format("{0}{1}-{2}{3}", startConsonants[0].ToString(), startConsonants[startConsonants.Length-1].ToString(), destConsonants[0].ToString(), destConsonants[destConsonants.Length-1].ToString()); 
    } 
} 
+9

принадлежит на codereview.stackexchange.com –

+4

Этот вопрос, как представляется, не по теме, поскольку речь идет о проверке кода. – MattDMo

+0

Это, похоже, было отправлено на проверку кода по адресу http://codereview.stackexchange.com/questions/39611/returning-consonants-of-a-planes-starting-and-ending-points – Sam

ответ

0

Вот некоторые предложения:

1 - Первая точка логично. Если вы передадите строку «Лондон-Ливан», вывод будет LN-LN, который я считаю недействительным. С вашей версией вы также можете получить сообщение об ошибке, если вы пройдете «Лондон-ОАЭ».

2 - Ваш конструктор требует аргумента, но непонятно, почему. Я предлагаю вам передать аргумент методу, который его использует, и проверить, действителен ли аргумент перед обработкой, как показано ниже.

3 - Разделение повторяется дважды без причины.

4 - Повторяйте ToString() несколько раз даже по строковым аргументам.

5 - Вы можете определить частный метод для форматирования. Это предотвращает ошибки в случае изменения правил.

Вот мой вариант включения вышеуказанных пунктов (за исключением первого):

class Flight 
    { 

     public string Consonants(string direction) 
     { 
      string[] words = direction.Split('-'); 
      // check if the separator is missing 
      if (words.Length!=2) return string.Empty; //missing separator error 

      return string.Format("{0}-{1}", Replacer(words[0]), Replacer(words[1])); 
     } 
     private string Replacer(string arg) 
     { 
      string abr= arg.ToUpper().Replace("A", string.Empty).Replace("E", string.Empty).Replace("I", string.Empty).Replace("O", string.Empty).Replace("U", string.Empty); 
      if (abr.Length < 2) 
       throw new Exception(string.Format("processing error in Replacer input {0} outpur {1}", arg, abr)); // example if you pass: UAE 

      return string.Format("{0}{1}", abr[0], abr[abr.Length - 1]); 
     } 

    } 
+0

Спасибо! Это очень полезно. – userr1311

+0

Я бы хотел, но он говорит, что у меня пока нет достаточной репутации, чтобы это сделать. – userr1311

+0

Благодарим вас за отзыв. – NoChance

0

Если вы не возражаете, верхний/нижний регистр различие, то вы могли бы использовать Linq для извлечения информации требуется

public string Consonants() 
{ 
    string starting = direction.Split('-')[0]; 
    string destination = direction.Split('-')[1]; 

    char[] cs = new char[] {'B','C','D','F','G','H','J','K','L','M','N','P','Q','R','S','T','V','W','X','Z'}; 
    string startFirst = starting.ToUpper().ToCharArray().First(x => cs.Contains(x)).ToString(); 
    string startLast = starting.ToUpper().ToCharArray().Last(x => cs.Contains(x)).ToString(); 
    string destFirst = destination.ToUpper().ToCharArray().First(x => cs.Contains(x)).ToString(); 
    string destLast = destination.ToUpper().ToCharArray().Last(x => cs.Contains(x)).ToString(); 
    return string.Format("{0}{1}-{2}{3}", startFirst, startLast, destFirst, destLast); 
} 
+0

Спасибо. Не могли бы вы рассказать мне, почему это лучше, чем код, который я написал? – userr1311

+0

@ userr1311 Если вы ищете информацию о качестве кода, вам будет больше удачи в [Code Review] (http://codereview.stackexchange.com). StackOverflow больше подходит для работы кода или нет. – TheEvilPenguin

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