Förbättring av SUB?

I grund och botten så är en DataReader det enda sättet att hämta data. Visst det finns outputparametrar också, men jag är tveksam till att det är gångbart med din databas. Vad använder du för DBMS?

GetResult är ett lite missvisande namn, eftersom inget returneras.

If objReader.HasRows Then
		objReader.Read()

kan förkortas till

If objReader.Read() Then

...och strängkonkatenering för att skapa en SQL-sats är en dålig ovana, även om det inte är någon direkt säkerhetsrisk i detta fall. Använd parametrar i stället.

Dessutom... tomsträngskonkatenering

Jag kan inget vb.net men det ser väl inte helt korrekt ut att skicka in en integer, myID och lägga den i sqlsträngen utan att konvertera den först.
Får man verkligen göra så i vb?! Tydligen, eftersom det funkar! Skumt i all fall! :)

doggelito skrev:

Jag kan inget vb.net men det ser väl inte helt korrekt ut att skicka in en integer, myID och lägga den i sqlsträngen utan att konvertera den först.
Får man verkligen göra så i vb?! Tydligen, eftersom det funkar! Skumt i all fall! :)

Och var konverteras den inte?

Sub myGetResult(ByVal sender As Object, ByVal e As System.Web.UI.WebControls.CommandEventArgs)
	myTools.GetResult(Convert.ToInt32(e.CommandArgument))
End Sub

:P

/edit Nu förstod jag vad du menade. Du konverterar inte i det läget. Det har inte med VB.NET att göra det har med databasen att göra. Det är väldigt viktigt att det du skickar in som värde faktiskt är av typen integer om nu kolumnen i databasen i det här fallet är av typen Numeric (access)

emission skrev:

Vad använder du för DBMS?

Access i det här fallet

emission skrev:

GetResult är ett lite missvisande namn, eftersom inget returneras.

Men det hämtas ju en fil från databasen som sedan öppnas i browsern, jag kan inte komma på nåt bättre. Har du några förslag? :)

emission skrev:

If objReader.Read() Then

Man får tacka, jag fick i min ungdom för mig att det inte gick eftersom det inte gick att göra en Read() om det inte fanns något att läsa!! Nu förstår jag hur det fungerar.

emission skrev:

...och strängkonkatenering för att skapa en SQL-sats är en dålig ovana, även om det inte är någon direkt säkerhetsrisk i detta fall. Använd parametrar i stället.

Det där är du inte den enda som påpekar, fattar inte varför jag inte bara slutar skriva dit det sista ingentinget!

Anledningen till att jag inte använder mig av parameteriserade frågor är för att det inte finns några snippets klara och jag tycker det tar för lång tid att skriva allting för hand.

Någon som vet hur man skapar egna snippets i VS2008??

doggelito skrev:

Får man verkligen göra så i vb?!

Det skulle kunna vara en vb-pryl, för vb har en massa dumt för sig med sin lösa typhantering, men i det här fallet är det faktiskt något du kan skriva i C# också. Operatoröverladdningen (sök "operator overloading") tar över och kör automatiskt ToString() på int-delen. Det är inte så snyggt, för man riskerar att slarva till det, men det funkar.

Man behöver inte köra vb med lös typhantering man kan köra vb i option strict!

Sub GetResult(ByVal myID As Integer)

	Dim connect As String = wbTools.myCon
	Dim objConn As New OleDbConnection(connect)
	objConn.Open()

	Dim cmd1 As OleDbCommand = objConn.CreateCommand()
	cmd1.CommandText = "SELECT FNamn, FStor, FBlob FROM tblSpelprogram where SmId=?"
	Dim p1 As New OleDbParameter()
	cmd1.Parameters.Add(p1)
	p1.Value = myID

	Dim objReader As OleDbDataReader = cmd1.ExecuteReader()

	Dim myName As String
	Dim mySize As Integer
	Dim myFile As Byte()

	While objReader.Read()
		myName = objReader("FNamn")
		mySize = objReader("FStor")
		myFile = objReader("FBlob")

		With HttpContext.Current.Response
			.ContentType = "text/html"
			.AddHeader("Content-Disposition", "filename=" & Trim(myName))
			.AddHeader("Content-Length", mySize)
			.BinaryWrite(myFile)
			.Flush()
		End With
	End While
	objReader.Close()
	objConn.Close()

End Sub

catz skrev:

cmd1.ExecuteReader()

Tror det skall finnas e ett commandBehaiver som gör att du endast tar en rad från databasen i din ExecuteReader().

Nu är det ingen jättevinst, men MS påstår att du skall tjäna in någon processorcyckel på att berätta för databasen redan vid frågeställningen att du endast tänker hämta en enda rad data från databasen, och inte som nu när det först är SQL som bestämmer hur många rader data som du skall få tillbaka....

Du skall flytta ner din conn.Open() till raden precis innan du gör din cmd.Execute() det finns ingen anledning att öppna databasen tidigare än dess, det försämrar bara applikationens skalbarhet. Likaså skall deklarationen av dina variabler komma in conn.open() och cmd.Executer(). När du har öppnat din koppling till databasen så skall du göra så lite som möjligt innan du stänger den igen... Det betyder också att hela HttpContext coden skall flyttas ner till efter du har stäng din databas, finns ingen anledning att göra den koden medans din databaskoppling är öppen, speciellt inte om du ändå tänker lägga värdena från databasen i egna variabler.

I vilket fall som helst så har du glömt felhanteringen, om du skulle få ett fel när du hämtar upp värden från databasen, så kommer din koppling till databasen aldrig att stängas. Så kaplsa in det i en try-catch() funktion, eller vad det heter i "hobbyspråket" ;)

- M

Gladh skrev:

Tror det skall finnas e ett commandBehaiver som gör att du endast tar en rad från databasen i din ExecuteReader().

Mycket riktigt,

Dim objReader As OleDbDataReader = cmd1.ExecuteReader(CommandBehavior.SingleRow)

Gladh skrev:

Du skall flytta ner din conn.Open() till raden precis innan du gör din cmd.Execute() det finns ingen anledning att öppna databasen tidigare än dess, det försämrar bara applikationens skalbarhet. Likaså skall deklarationen av dina variabler komma in conn.open() och cmd.Executer(). När du har öppnat din koppling till databasen så skall du göra så lite som möjligt innan du stänger den igen... Det betyder också att hela HttpContext coden skall flyttas ner till efter du har stäng din databas, finns ingen anledning att göra den koden medans din databaskoppling är öppen, speciellt inte om du ändå tänker lägga värdena från databasen i egna variabler.

objConn.Open()
Dim objReader As OleDbDataReader = cmd1.ExecuteReader(CommandBehavior.SingleRow)

While objReader.Read()
	myName = objReader("FNamn")
	mySize = objReader("FStor")
	myFile = objReader("FBlob")
	objReader.Close()
	objConn.Close()

	With HttpContext.Current.Response
		.ContentType = "text/html"
		.AddHeader("Content-Disposition", "filename=" & Trim(myName))
		.AddHeader("Content-Length", mySize)
		.BinaryWrite(myFile)
		.Flush()
	End With
End While

Gladh skrev:

I vilket fall som helst så har du glömt felhanteringen, om du skulle få ett fel när du hämtar upp värden från databasen, så kommer din koppling till databasen aldrig att stängas. Så kaplsa in det i en try-catch() funktion, eller vad det heter i "hobbyspråket" ;)

Hobbyspråk! :P Själv är jag inte alls förtjust i Try - Catch, det brukar ta sån errm jäkla tid innan jag hittar vad problemet är. Finns det några andra sätt att hantera fel? Hur gör ni i C#? Jag markerar tråden som olöst igen, det finns utrymme för mer diskussion här.

Tid innan du hittar felet? Kanske du förstör din stacktrace när du bubblar upp felen, hur fångar och kastar du dina exceptions

Felhantering är a och o i en applikation. Det beror på vad det är för exception och vart det sker.

http://msdn2.microsoft.com/en-us/library/seyhszts.aspx
http://www.codeproject.com/dotnet/exceptionbestpractices.asp

erka skrev:

Tid innan du hittar felet? Kanske du förstör din stacktrace när du bubblar upp felen, hur fångar och kastar du dina exceptions

Felhantering är a och o i en applikation. Det beror på vad det är för exception och vart det sker.

http://msdn2.microsoft.com/en-us/library/seyhszts.aspx
http://www.codeproject.com/dotnet/exceptionbestpractices.asp

Ska jag ta hand om felen redan från början? Jag fick för mig att man börjar ta hand om felen när applikationen närmar sig färdig och man inleder testningsfasen, dvs då man går igenom varje del av den och försöker "ha sönder den". När det är klart så brukar jag säkra med Try Catch Finally och då göra något med felen.

Är det fel tänkt?

Nej man ska utveckla sin applikation med felhantering från start, där det kan gå fel. Läs igenom artiklarna, att du har svårt att hitta vart felen sker kanske beror på att du någon stan kastar execptionet vidare och inte endast använder throw, vilket förstör stacktracen

erka skrev:

Nej man ska utveckla sin applikation med felhantering från start, där det kan gå fel. Läs igenom artiklarna, att du har svårt att hitta vart felen sker kanske beror på att du någon stan kastar execptionet vidare och inte endast använder throw, vilket förstör stacktracen

Till att börja med, tack så jättemycket för länkarna det var ta mej tusan den bästa läsningen jag haft om exceptions!!

Jag läser på codeproject nu. Man ska alltså bara kasta det / de felen som är aktuella, man ska spara det någonstanns men bara en gång för varje fel. Ska jag då söka igenom log(txt) filen efter exakt samma fel och inte skriva till textfilen om felet finns?

Hur gör folk med felhanteringen egentligen?

catz skrev:

objConn.Open()
Dim objReader As OleDbDataReader = cmd1.ExecuteReader(CommandBehavior.SingleRow)

While objReader.Read()
myName = objReader("FNamn")
mySize = objReader("FStor")
myFile = objReader("FBlob")
objReader.Close()
objConn.Close()

With HttpContext.Current.Response
.ContentType = "text/html"
.AddHeader("Content-Disposition", "filename=" & Trim(myName))
.AddHeader("Content-Length", mySize)
.BinaryWrite(myFile)
.Flush()
End With
End While

Nja... det ser väl inte så rätt ut, gör det?

objConn.Open()
Dim objReader As OleDbDataReader = cmd1.ExecuteReader(CommandBehavior.SingleRow)

While objReader.Read()
	myName = objReader("FNamn")
	mySize = objReader("FStor")
	myFile = objReader("FBlob")
End While
objReader.Close()
objConn.Close()

With HttpContext.Current.Response
	.ContentType = "text/html"
	.AddHeader("Content-Disposition", "filename=" & Trim(myName))
	.AddHeader("Content-Length", mySize)
	.BinaryWrite(myFile)
	.Flush()
End With

Blir nog mycket bättre... Du kanske skall ha en koll så dina variabler verkligen innehåller något värde innan du skriver de till responsestreamen...

catz skrev:

Man ska alltså bara kasta det / de felen som är aktuella, man ska spara det någonstanns men bara en gång för varje fel. Ska jag då söka igenom log(txt) filen efter exakt samma fel och inte skriva till textfilen om felet finns?

Nej, det är inte så de menar, de menar att du inte skall göra 10 try-catch i varandra där du i varje try-catch skriver ut samma fel. Om du råkar få samma fel men vid 2 helt olika tillfällen, så skall båda givetviss loggas till felloggen, eftersom det är helt skilda fel (även om det råkar vara samma feltyp).

catz skrev:

Hur gör folk med felhanteringen egentligen?

Det är nog väldigt olika, men generellt så kan man säga, skall du inte göra något med felet du fångar, så behöver du inte fånga det heller.

Det finns ingen mening med att skriva en massa try-catch funktioner om du ändå bara tänker kasta felet vidare upp, om du däremot skall göra något speciellt om det inträffar ett fel, så skall du fånga felet, göra vad du skall, och låta applikationen gå vidare, eller möjligtviss kasta felet vidare upp.

- M

Gladh skrev:

Det finns ingen mening med att skriva en massa try-catch funktioner om du ändå bara tänker kasta felet vidare upp, om du däremot skall göra något speciellt om det inträffar ett fel, så skall du fånga felet, göra vad du skall, och låta applikationen gå vidare, eller möjligtviss kasta felet vidare upp.

- M

Då är vi tillbaka på det jag höll på med att försöka se till så inte felen händer till att börja med! Men jag bör absolut använda mig av Try/Catch för att inte lämna databasanslutningarna öppna jämnt.

Jag har nog tidigare varit dålig på att stänga mina object.

Gladh skrev:

Blir nog mycket bättre... Du kanske skall ha en koll så dina variabler verkligen innehåller något värde innan du skriver de till responsestreamen...

Någon som har en bra funktion för att kontrollera om något är tomt, null eller nothing? Jag har gjort en funktion för det men vet inte om den är särskilt vettig.

Public Function IsBlank(ByVal myValue As String) As Boolean
	IsBlank = False

	If IsNothing(myValue) Then
		IsBlank = True
	ElseIf IsDBNull(myValue) Then
		IsBlank = True
	ElseIf myValue.GetType Is GetType(System.String) Then
		If Trim(myValue) = "" Then
			IsBlank = True
		End If
	End If

End Function

CatZ skrev:

Men jag bör absolut använda mig av Try/Catch för att inte lämna databasanslutningarna öppna jämnt.

För att se till att databasanslutningen stängs efter användadet oberoende på om fel uppstår eller ej kan du använda Using.

catz skrev:

Någon som har en bra funktion för att kontrollera om något är tomt, null eller nothing? Jag har gjort en funktion för det men vet inte om den är särskilt vettig.

i C# så har du String.IsNullorNothing(mystring), det kanske finns något liknande i "hobbyspråket" ;)

- M

Ähum... den heter väl snarare String.IsNullOrEmpty(mystring) :)

(hur kan man låta bli att rätta Gladh när man en gång fått läget?? :e )

frequz skrev:

Ähum... den heter väl snarare String.IsNullOrEmpty(mystring)

Det gör den säkert.. har ingen aning om vad den heter, vet bara att den finns där och så sköter intillisensen resten :birp

- M