xsharp.eu • Is the code correct? - Page 2
Page 2 of 2

Is the code correct?

Posted: Thu Oct 20, 2016 1:47 pm
by Frank Maraite
Hi Jacek,

in additiopn to the comments on Var naming, I would refactor to more story telling method names. It feels like working with a hammer, but you will like it later when rereading.

The first step is: init LOCAL's when just before needed:

METHOD fbLoadData(sSQL AS STRING) AS VOID

LOCAL dsHistoria AS DataSet
dsHistoria := DataSet{}

LOCAL daHistoria AS FbDataAdapter
daHistoria := FbDataAdapter{sSQL, SELF:fbConn}
daHistoria:Fill(dsHistoria)

LOCAL bsHistoria AS BindingSource
bsHistoria := BindingSource{}
bsHistoria:DataSource := dsHistoria:Tables[0]

SELF:oBindingNavigator1:BindingSource := bsHistoria
SELF:odgvHistoria:DataSource := bsHistoria

This way you clearly see when and why they are needed.

Then replace all! instantiation of objects/classes with factory method's.

You may put additional logic in and do method names like comments.
Try to avoid class names in var names. It's not easy here.

You see: the method name say, this method is responsible to two things: loading data and binding to GUI. It has an 'and' in it's name. That's too much. Loading data and connecting to GUI are two completly different things. Do this in two decent steps.

For now I think this shows better what you are doing. It has short method's, is testable, story telling methods and meaningful names.

Just m 2 ct's

Frank

METHOD LoadHistoriaAndBindToGUI( HistoriaDataBaseName AS STRING) AS VOID

LOCAL HistoriaData AS DataSet
HistoriaData := GetDataSet() // Never ever instantiate objects directly !!!

FillHistoriaWithData( HistoriaDataBaseName, HistoriaData )

LOCAL BindingSourceToHistoriaFirstTable AS BindingSource
BindingSourceToHistoriaFirstTable := CreateBindingSourceToHistoriaFirstTable( HistoriaData )

SetBindingToNavigatorAndDataGrid( BindingToHistoriaFirstTable )

INTERNAL METHOD GetDataSet() AS DataSet // Factory Method
RETURN DataSet{}

INTERNAL METHOD FillHistoriaWithData( HistoriaDataBaseName AS STRING, HistoriaDataSet AS DataSet ) AS VOID
LOCAL HistoriaDataAdapter AS FbDataAdapter
HistoriaDataAdapter := FbDataAdapter{ HistoriaDataBaseName AS STRING, SELF:fbConn AS ???}
HistoriaDataAdapter :Fill(HistoriaDataSet )

INTERNAL METHOD CreateBindingToHistoriaFirstTable( HistoriaDataSet AS DataSet ) AS VOID
BindingToHistoriaFirstTable := BindingSource{}
BindingToHistoriaFirstTable :DataSource := HistoriaDataSet :Tables[0]

INTERNAL METHOD SetBindingToNavigatorAndDataGrid( BindingToHistoriaFirstTable AS BindingSource) AS VOID
SELF:HistoriaBindingNavigator:BindingSource := BindingToHistoriaFirstTable
SELF:HistoriaDataGrid:DataSource := BindingToHistoriaFirstTable