Verbesserunsvorschläge

[cxg5z]

Hallo,

hab mich mal an ein kleines Makro getraut, gibt es hier noch etwas zu verbessern?

[WillWissen]

Hi,

diesen Teil

intErsteLeereZeile = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row + 1
    ActiveSheet.Cells(intErsteLeereZeile, 1).Value = Me.txtDatum.Value
    ActiveSheet.Cells(intErsteLeereZeile, 2).Value = Me.txtbenzin.Value
    ActiveSheet.Cells(intErsteLeereZeile, 3).Value = CCur(Me.txtPreis.Value)
    ActiveSheet.Cells(intErsteLeereZeile, 4).Value = CCur(Me.txtKilometer.Value)
    ActiveSheet.Cells(intErsteLeereZeile, 5).Value = Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100
    ActiveSheet.Cells(intErsteLeereZeile, 6).Value = CCur(Me.txtPreis.Value) / Me.txtbenzin.Value * Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100 / 100
    ActiveSheet.Cells(intErsteLeereZeile, 7).Value = CCur(Me.txtPreis.Value) / Me.txtbenzin.Value
    ActiveSheet.Cells(intErsteLeereZeile, 8).Value = Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100 / 100

kannst du so verkürzen:

intErsteLeereZeile = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row + 1
    With ActiveSheet
        .Cells(intErsteLeereZeile, 1).Value = Me.txtDatum.Value
        .Cells(intErsteLeereZeile, 2).Value = Me.txtbenzin.Value
        .Cells(intErsteLeereZeile, 3).Value = CCur(Me.txtPreis.Value)
        .Cells(intErsteLeereZeile, 4).Value = CCur(Me.txtKilometer.Value)
        .Cells(intErsteLeereZeile, 5).Value = Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100
        .Cells(intErsteLeereZeile, 6).Value = CCur(Me.txtPreis.Value) / Me.txtbenzin.Value * Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100 / 100
        .Cells(intErsteLeereZeile, 7).Value = CCur(Me.txtPreis.Value) / Me.txtbenzin.Value
        .Cells(intErsteLeereZeile, 8).Value = Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100 / 100
    End With

[Rabe]

Hi Günter,

evtl. das "intErsteLeereZeile" auch noch in die With einschließen.

Code:

    With ActiveSheet
        intErsteLeereZeile = .Cells(Rows.Count, 1).End(xlUp).Row + 1
       .Cells(intErsteLeereZeile, 1).Value = Me.txtDatum.Value
...
    End With

Und "Option Explicit" am Anfang jedes Moduls (Tabellen oder Formulare oder allgemeines Modul) einfügen

[WillWissen]

Hi,

was mir noch aufgefallen ist:

du lässt teilweise deine Zahlen als Text schreiben.

[
Bild bitte so als Datei hochladen: Klick mich!
]

Damit kannst du nicht weiterrechnen (z.B. Gesamtsummen bilden)

[RPP63]

Hi Ralf!

evtl. das "intErsteLeereZeile" auch noch in die With einschließen

Kann man machen, aber auch anders:  :21:

Code:

With ActiveSheet.Rows(intErsteLeereZeile)
  .Cells(1) = txtDatum
  .Cells(2) = txtbenzin
  '...
End With

Spart Bildschirmtinte, ist aber nicht unbedingt Ernst gemeint.  :19:

Gruß Ralf

[Rabe]

Hi Ralf,

Spart Bildschirmtinte, ist aber nicht unbedingt Ernst gemeint.  :19:

aber das gefällt mir!

Außerdem: gefahrene Kilometer sind keine Währung!

Also so:

Private Sub cmdEingabe_Click()
   'fügt angaben ins tabellenblatt ein 
   
   Dim intErsteLeereZeile As Long
   
   intErsteLeereZeile = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row + 1
   With ActiveSheet.Rows(intErsteLeereZeile)
      .Cells(1) = txtDatum
      .Cells(2) = txtbenzin * 1
      .Cells(3) = CCur(txtPreis)
      .Cells(4) = txtKilometer * 1
      .Cells(5) = txtbenzin / CCur(txtKilometer) * 100
      .Cells(6) = CCur(txtPreis) / txtbenzin * txtbenzin / CCur(txtKilometer) * 100 / 100
      .Cells(7) = CCur(txtPreis) / txtbenzin * 1
      .Cells(8) = txtbenzin / CCur(txtKilometer) * 100 / 100
   End With
   
End Sub

[cxg5z]

Danke erstmal für die Hinweise,
das mit den als Text gespeichrten Zahlen macht natürlich Sinn...
werde es natürlich ändern

LG Micha

[RPP63]

Und an den TE:

1. Du referenzierst auf das Userform mittels Me. Das ist i.O., aber überflüssig.
   
2. Du schließt mit Unload frmbenzin, hier würde jetzt Me reichen Unload Me
   
3. Du benutzt den default .Value, i.O. aber ebenfalls überflüssig.
   
4. Du versuchst, bei der Übergabe den Text der txtBoxes mittels CCur() in ein Währungsformat umzuwandeln. Prinzipiell gut, bloß was machst Du, wenn mal kein umwandelbarer Text drin steht? Viel besser ist es, eine Prüfung ins jeweilige txtBox_Exit zu schreiben, ansonsten schmiert Dir Dein Code ab.
   
5. Deine Division könnte zu einem DIV/0 führen, dies ist nicht sauber.
   

Fazit: Dein Code funktioniert nur, wenn der Bediener keinen Fehler macht.
Dies solltest Du Dir gar nicht erst angewöhnen.

Ich stelle später mal einen Code ein, wie ich ihn schreiben würde.

Gruß Ralf

[RPP63]

Zwischenstand, während ich schreibe:
Statt:

ActiveSheet.Cells(intErsteLeereZeile, 5).Value = Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100
ActiveSheet.Cells(intErsteLeereZeile, 6).Value = CCur(Me.txtPreis.Value) / Me.txtbenzin.Value * Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100 / 100

geht durchaus auch:

.Cells(5) = txtbenzin / txtKilometer * 100
.Cells(6) = .Cells(3) / .Cells(5)

:19:

Bis später,

Ralf

[BoskoBiati]

Hallo,


und was ist das für eine Rechnung:

Code:

CCur(Me.txtPreis.Value) / Me.txtbenzin.Value * Me.txtbenzin.Value / CCur(Me.txtKilometer.Value) * 100 / 100

in Zahlen: 1,169*10*10/200*10/10 =1,69 *10^2 / 200 * 1 = 0,845

wieso Kilometer in Währung umwandeln?