2010-10-18 17 views
7

Witam wszystkich, to jest mój mały kod Frankensteina, nie żartujcie z tego, to działa! Przekazywałbyś więc nazwę tabeli i dane jako tablicę asocjacyjną, które są obiektami. Jestem prawie pewien, że to nie jest dobry kod, tak jak byłem i nadal uczę się ActionScript. Więc co mogę zmienić lub jak moglibyście zrobić to lepiej?Jak mogę poprawić mój kod AS3/Air?

public function save(table:String,data:Object):void 
     { 
      var conn:SQLConnection = new SQLConnection(); 
      var folder:File = File.applicationStorageDirectory; 
      var dbFile:File = folder.resolvePath("task.db"); 
      conn.open(dbFile); 

      var stat:SQLStatement=new SQLStatement(); 
      stat.sqlConnection=conn; 

      //make fields and values 
      var fields:String=""; 
      var values:String=""; 
      for(var sRole:String in data) 
      { 
       fields=fields+sRole+",:"; 
       stat.parameters[":"+sRole]=data[sRole]; 
      } 
      //trim off white space 
      var s:String=new String(fields); 
      var cleanString:String=s.slice(0, -2); 

      //over here we add : infront of the values I forget why 
      var find:RegExp=/:/g; 
      var mymyField:String=new String(cleanString.replace(find,"")); 
      cleanString=":"+cleanString; 

      var SQLFields:String=mymyField; 
      var SQLValues:String=cleanString; 

      stat.text="INSERT INTO "+table+" ("+SQLFields+")VALUES("+SQLValues+")"; 

      stat.execute(); 
     } 

Odpowiedz

5

Część, w której budujesz zapytanie, jest dość przerażająca, szczerze mówiąc. Połowa kodu usuwa śmieci, które dodałeś zaledwie kilka linii wcześniej. To sprawia, że ​​trudno jest je przeczytać i zrozumieć. Co jest oznaką niskiej jakości kodu. Poniższe jest znacznie krótsze i prostsze:

 //make fields and values 
     var fields:Array = []; 
     for(var field:String in data) { 
      fields.push(field); 
      stat.parameters[":"+field]=data[fieldName]; 
     } 
     var sqlFields:String = fields.join(","); 
     var sqlValues:String = ":"+fields.join(",:"); 

     stat.text="INSERT INTO "+table+" ("+sqlFields+")VALUES("+sqlValues+")"; 

     stat.execute(); 
3

Ktoś kiedyś powiedział mi, że głupi pomysł, który działa, nie jest głupi. Pierwszym naszym celem programisty jest (często) rozwiązywanie problemów biznesowych; i dopóki nasz kod to robi, to nam się to udaje. Nie musisz przepraszać za działający kod.

Pod względem tego, co zrobiłbym, aby zmienić Twój fragment; Mogę po prostu zamknąć to jeszcze bardziej. Czy folder, dbFile i nazwa pliku db (task.db) mogą zostać dodane jako właściwości do klasy lub do argumentów metody?

Czy można oddzielić tworzenie instrukcji SQL od obsługi połączenia od analizy danych?

+0

Zgodziłbym się z tym pierwszym stwierdzeniem, o ile zasoby systemu nie są nadużywane. Pamięć, zwłaszcza w pamięci flash przeglądarki, powinna być zawsze zarządzana poprawnie. – TheDarkIn1978

+0

+1. chociaż kod jest naprawdę tak okropny, że nie powiedziałbym, że faktycznie "działa" jak kod. – back2dos

3

Kilka uwag,

  • Jak powiedział, zanim będzie można wydzielić wszystkie połączenia db tak można ponownie użyć funkcji bez przepisywania go jeśli trzeba zmienić nazwę dB.

  • Nie używaj nowy String() można go uniknąć

  • nie jest przydatny do czyszczenia białą przestrzeń pomiędzy swojej dziedzinie: A: B jest taka sama, jak: a,: b

  • Niektóre konwencja nie rozpocząć swoją lokalną nazwę var z dużymi literami, a to nie jest przydatny do przypisać je do innego var

Jeśli nie dostać się źle po tym jak twój //make fields and values mogą być zapisane na przykład:

//make fields and values 
var fields:String = ""; 
var values:String = ""; 
var fieldSeparator:String = ""; 

for(var sRole:String in data) 
{ 
    fields += fieldSeparator + sRole; 

    var paramName:String = ":" + sRole; 
    values += fieldSeparator + paramName; 

    stat.parameters[paramName] = data[sRole]; 

    fieldSeparator = ","; 
} 

stat.text = "INSERT INTO " + table +" (" + fields + ") VALUES (" + values + ")"; 

stat.execute(); 
Powiązane problemy