|
I found this piece of VeryBAd VBA code in an Excel spreadsheet I was amending today:
For counter = avgfirstrow To avglastrow
cellvalue = Sheets(1).Cells(counter, 3).Value
If cellvalue < 0 Then GoTo ignore
If cellvalue > 99 Then GoTo ignore
totalsum = totalsum + cellvalue
GoTo nextone
ignore:
entries = entries - 1
nextone:
Next
I can assure you my left eyebrow is still twitching.
Gibber gibber.... 
My only guess is that Satan must have been whispering into my colleague's ear at the time.
You always pass failure on the way to success.
|
|
|
|
|
Is your anger mainly directed to VBA or to your colleague's code? What if your colleague wrote the following instead?
For counter = avgfirstrow To avglastrow
cellvalue = Sheets(1).Cells(counter, 3).Value
If cellvalue < 0 Or cellvalue > 99 Then entries = entries - 1
Else totalsum = totalsum + cellvalue
Next
The reason I ask is many people seem to be happy with denouncing anything with VB in it, not realizing that it is possible to write good code in VB/VBA, just like it is possible to write bad code in C++/C#.
Thanks.
|
|
|
|
|
Xiangyang Liu wrote: Is your anger mainly directed to VBA or to your colleague's code?
My colleagues lack of coding proficiency(What a Scrooge I am and it's Chistmas tomorrow).
Unfortunately I have not been supervising his coding enough.
It's a tricky area as happy coders make better coders - so I try to have a light touch when it comes to others code.
Although I do like to have a good old moan on CodeProject occasionally.
Xiangyang Liu wrote: it is possible to write good code in VB/VBA, just like it is possible to write bad code in C++/C#.
I agree.
My experience is that VBA encourages bad techniques and habits.
To code in C# you have to have a relatively good understanding of programming methodology(am I wrong?).
You can throw anyone at VBA and it will throw something back up.
Xiangyang Liu wrote: What if your colleague wrote the following instead?
I would have had nothing to complain about except for the formatting(there I go again).
Merry Christmas 
Guy
You always pass failure on the way to success.
|
|
|
|
|
GuyThiebaut wrote: To code in C# you have to have a relatively good understanding of programming methodology(am I wrong?).
True. VB family supports unstructured error handling like On Error Resume Next but such constructs are totally not supported in C#.
Vasudevan Deepak Kumar
Personal Homepage
Tech Gossips
A pessimist sees only the dark side of the clouds, and mopes; a philosopher sees both sides, and shrugs; an optimist doesn't see the clouds at all - he's walking on them. --Leonard Louis Levinson
|
|
|
|
|
I agree with you. Is the average VB programmer who brought discredit on the language.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
[my articles]
|
|
|
|
|
"Omit needless local variables." -- Strunk... if he'd taught programming
(cellvalue in this case)
|
|
|
|
|
Thanks - I missed that one as well.
Merry Christmas
You always pass failure on the way to success.
|
|
|
|
|
Actually his usage of a private variable was correct, because otherwords he would have to keep drilling down the object model to get the value in each conditional, and it's better to incur the performance overhead of allocating a new variable once than to keep traversing the object model more than once in a loop like that. "Real" VB (which is NOT the same as VBA) gives you the With statement which prevents the need for this additional variable. What he did actually makes the code more readable as well.
Keep in mind that VBA is meant for simple macros, so there is really no horror in what he did, even with macros. Gotos are considered bad because they lead to spaghetti code smells in applications that have grown beyond a particular size, but used in a small macro situation they can greatly enhance readability if used properly.
I'm not trying to wave the VB banner, I actually bailed on it quite some time ago only using it when I have to, simply because most good developers chose to focus on C#, thus it's easier to go with the flow and I like C-like syntaxes.
But most VB-bigots are not as smart as they think they are and would feel quite stupid for making blanket statements about VB if they knew all the facts. I've seen really amazing, robust, well-performing apps written in both VB6 and VB.Net, and VB itself incorporates many elements of Pascal which is an excellent language.
I do agree that the relative simplicity of getting started with VB led to an influx of bad programmers (many of which got flushed out in the dot com bust), or people who were uneducated in formal CS concepts or simply were not technical or motivated enough to make a career committment to good programming, but at the same time I've worked with some real idiots whose background was in Java or C++. I always thought Microsft screwed up by keeping the whole "BASIC" moniker... when they came out with .Net they should have just named it B++ or B# or something that made more sense, because despite a few syntax differences it is much closer to C# than it is to the original BASIC.
|
|
|
|
|
Tipster wrote:
Keep in mind that VBA is meant for simple macros
Not in my programming experience.
I have had to use VBA for some fairly sophisticated image manipulation in Word as well as using it extensively in Access and Excel.
Tipster wrote: Gotos are considered bad... but used in a small macro situation they can greatly enhance readability if used properly
I'd like to see an example of that.
Ok I have used goto's (Oh I know the shame of it  ) but really they should never be used if there is a better alternative.
When I was at University the only place a goto was ever allowed was in certain situations in COBOL.
I was taught to program properly through PASCAL and I think many people new to programming would benefit from this sort of education - i.e. structured programming.
Alright - rant over 
You always pass failure on the way to success.
|
|
|
|
|
Xiangyang Liu wrote: that it is possible to write good code in VB/VBA, just like it is possible to write bad code in C++/C#.
But it is easier to write bad code in C++/C#, than it is to write good code in VB/VBA! 
|
|
|
|
|
Your code is in error. You cannot have the "Else" clause on a separate line if the "Then" clause is on the same line as the condition.
For counter = avgfirstrow To avglastrow
cellvalue = Sheets(1).Cells(counter, 3).Value
If cellvalue < 0 or cellvalue > 99 Then
entries = entries - 1
Else
totalsum = totalsum + cellvalue
End If
Next
|
|
|
|
|
Where are the Goto statements in C++/C#??
Matt
(Find your own niche! This one's mine.)
|
|
|
|
|
Matt Sollars wrote: Where are the Goto statements in C++/C#??
Surely that is an oxymoron
You always pass failure on the way to success.
|
|
|
|
|
Matt
(Find your own niche! This one's mine.)
|
|
|
|
|
Yikes! Why not put entries = entries - 1 after Then 
Also just have one if statement 
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
|
|
|
|
|
Hmmm, that's pretty good compared to some of the code I've had to deal with 
GuyThiebaut wrote: I found this piece of VeryBAd VBA code in an Excel spreadsheet I was amending today:
For counter = avgfirstrow To avglastrow cellvalue = Sheets(1).Cells(counter, 3).Value If cellvalue < 0 Then GoTo ignore If cellvalue > 99 Then GoTo ignore totalsum = totalsum + cellvalue GoTo nextoneignore: entries = entries - 1nextone:Next
I can assure you my left eyebrow is still twitching.
Gibber gibber....
My only guess is that Satan must have been whispering into my colleague's ear at the time.
|
|
|
|
|
I guess it makes sense, and works, in it's own eccentric manner.
You always pass failure on the way to success.
|
|
|
|
|
My company has a well organized naming rule.
It's a standard which has been developed for a few years.
Look:
Table names like: TB200010
View names like: VI600230
Procedure names like: USP980010_07R
Web services names like: WS210080.asmx
Web form page names like: UI400740.aspx
Reporting service page names like: RPT1731.aspx
There are thousands of objects like the aboves.
Here the example (real one):
// ????
public class UI200020
{
// ??? ??? ??
string TabPageUrl01 = ...;
string TabPageUrl02 = ...;
string TabPageUrl03 = ...;
string TabPageUrl04 = ...;
string TabPageUrl05 = ...;
// ??? ??
private WS980010.XD980010 xd980010UserInfo;
// ??? ????
WS200010 wsProxy = ...;
WS100010 wsProxy2 = ...;
// ? ???
protected System.Web.UI.WebControls.DataGrid dgListSub01;
protected System.Web.UI.WebControls.DataGrid dgListSub02;
protected System.Web.UI.WebControls.DataGrid dgListSub03;
protected System.Web.UI.WebControls.DataGrid dgListSub04;
protected System.Web.UI.WebControls.DataGrid dgListSub05;
protected System.Web.UI.WebControls.DataGrid dgListSub06;
protected System.Web.UI.WebControls.DataGrid dgListSub07;
protected System.Web.UI.WebControls.DataGrid dgListSub09;
protected System.Web.UI.WebControls.DataGrid dgListSub10;
protected System.Web.UI.WebControls.DataGrid dgListSub11;
protected System.Web.UI.WebControls.DataGrid dgListSub12;
// ???? ?? ?? ??
protected System.Web.UI.WebControls.ImageButton ibtnTabMore01;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore02;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore03;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore04;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore05;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore06;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore09;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore10;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore11;
protected System.Web.UI.WebControls.ImageButton ibtnTabMore12;
// ? ?? ??
protected System.Web.UI.WebControls.Label lblSUBTITLE01;
protected System.Web.UI.WebControls.Label lblSUBTITLE02;
protected System.Web.UI.WebControls.Label lblSUBTITLE03;
protected System.Web.UI.WebControls.Label lblSUBTITLE04;
protected System.Web.UI.WebControls.Label lblSUBTITLE05;
protected System.Web.UI.WebControls.Label lblSUBTITLE11;
protected System.Web.UI.WebControls.Label lblTABTITLE06;
protected System.Web.UI.WebControls.Label lblTABTITLE10;
protected System.Web.UI.WebControls.Label lblTABTITLE11;
// ??? ??
protected System.Web.UI.WebControls.Panel PN0;
protected System.Web.UI.WebControls.Panel PN1;
protected System.Web.UI.WebControls.Panel PN10;
protected System.Web.UI.WebControls.Panel PN11;
protected System.Web.UI.WebControls.Panel PN2;
protected System.Web.UI.WebControls.Panel PN3;
protected System.Web.UI.WebControls.Panel PN4;
protected System.Web.UI.WebControls.Panel PN5;
protected System.Web.UI.WebControls.Panel PN6;
protected System.Web.UI.WebControls.Panel PN9;
#region ? More ?? ??
private void ibtnTabMore01_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub01);
this.GetData(this.Key);
}
private void ibtnTabMore02_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub02);
this.GetData(this.Key);
}
private void ibtnTabMore04_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub04);
this.GetData(this.Key);
}
private void ibtnTabMore05_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub05);
this.GetData(this.Key);
}
private void ibtnTabMore03_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub03);
this.GetData(this.Key);
}
private void ibtnTabMore06_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub06);
this.GetData(this.Key);
}
private void ibtnTabMore10_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub10);
this.GetData(this.Key);
}
private void ibtnTabMore09_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub09);
this.GetData(this.Key);
}
private void ibtnTabMore11_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub11);
this.GetData(this.Key);
}
private void ibtnTabMore12_Click(object sender, System.Web.UI.ImageClickEventArgs e)
{
// ?? DataGrid? ???? ? ???? ??
this.MorePageSize(this.dgListSub12);
this.GetData(this.Key);
}
#endregion
// ? ??? ???? ???
if (xd200010Set.XD280010S != null &&
xd200010Set.XD280010S.Length > 0)
{
this.dgListSub01.DataSource = CreateDataSource(xd200010Set.XD280010S) ;
this.dgListSub01.DataBind();
}
//???? ?
if (xd200010Set.XD110010S != null &&
xd200010Set.XD110010S.Length > 0)
{
this.dgListSub12.DataSource = CreateDataSource(xd200010Set.XD110010S) ;
this.dgListSub12.DataBind();
// ???? ?? & Tooltip ??.
foreach(DataGridItem item in dgListSub12.Items)
{
string rawStr = ((Label)item.Cells[4].FindControl("lblCTTDESC")).Text;
if(rawStr.Length > 200)
{
item.Cells[4].ToolTip = rawStr.Substring(0, 200) ;
}
else
{
item.Cells[4].ToolTip = rawStr ;
}
if( rawStr.Length > 34)
item.Cells[4].Text = rawStr.Substring(0,34)+" ...";
}
}
if (xd200010Set.XD210010S != null &&
xd200010Set.XD210010S.Length > 0)
{
this.dgListSub02.DataSource = CreateDataSource(xd200010Set.XD210010S) ;
this.dgListSub02.DataBind();
// ????, ??? ???? ?? & Tooltip ??.
foreach(DataGridItem item in dgListSub02.Items)
{
Label tlblSALENAME = ((Label)item.FindControl("lblSALENAME"));
Label tlblPRDID_ = ((Label)item.FindControl("lblPRDID_"));
string rawSALENAME = tlblSALENAME.Text;
string rawPRDID_ = tlblPRDID_.Text;
tlblSALENAME.ToolTip = rawSALENAME;
tlblPRDID_.ToolTip = rawPRDID_;
if( rawSALENAME.Length > 20)
tlblSALENAME.Text = rawSALENAME.Substring(0,20)+" ...";
if( rawPRDID_.Length > 20)
tlblPRDID_.Text = rawPRDID_.Substring(0,20)+" ...";
}
}
if (xd200010Set.XD220020S != null &&
xd200010Set.XD220020S.Length > 0)
{
this.dgListSub05.DataSource = CreateDataSource(xd200010Set.XD220020S) ;
this.dgListSub05.DataBind();
xd220020AMT = wsPurBill.ReadAMT(Key, QUERYUSER);
xd220020NAMT = wsPurBill.ReadNAMT(Key, QUERYUSER);
this.lblCNT2.Text = xd220020AMT.RCOUNT ;
this.lblAMT2.Text = xd220020AMT.TOTAMT ;
this.lblNCNT2.Text = xd220020NAMT.NRCOUNT;
this.lblNAMT2.Text = xd220020NAMT.NTOTAMT;
}
if (xd200010Set.XD220010S != null &&
xd200010Set.XD220010S.Length > 0)
{
this.dgListSub04.DataSource = CreateDataSource(xd200010Set.XD220010S) ;
this.dgListSub04.DataBind();
xd220010AMT = wsSaleBill.ReadAMT(Key, QUERYUSER);
xd220010NAMT = wsSaleBill.ReadNAMT(Key, QUERYUSER);
this.lblCNT1.Text = xd220010AMT.RCOUNT ;
this.lblAMT1.Text = xd220010AMT.TOTAMT ;
this.lblNCNT1.Text = xd220010NAMT.NRCOUNT ;
this.lblNAMT1.Text = xd220010NAMT.NTOTAMT ;
}
if (xd200010Set.XD230030S != null &&
xd200010Set.XD230030S.Length > 0)
{
this.dgListSub06.DataSource = CreateDataSource(xd200010Set.XD230030S) ;
this.dgListSub06.DataBind();
xd230030AMT = wsOrder.ReadAMT(Key, QUERYUSER, "CLIENT");
this.lblCNT4.Text = xd230030AMT.RCOUNT ;
this.lblAMT4.Text = xd230030AMT.TOTAMT ;
this.lblPOINT4.Text = xd230030AMT.TOTALPOINT ;
}
if (xd200010Set.XD230010S != null &&
xd200010Set.XD230010S.Length > 0)
{
this.dgListSub10.DataSource = CreateDataSource(xd200010Set.XD230010S) ;
this.dgListSub10.DataBind();
xd230010AMT = wsEst.ReadAMT(Key, QUERYUSER, "CLIENT");
this.lblCNT3.Text = xd230010AMT.RCOUNT ;
this.lblAMT3.Text = xd230010AMT.TOTAMT ;
}
if (xd200010Set.FILES != null &&
xd200010Set.FILES.Length > 0)
{
this.dgListSub03.DataSource= CreateDataSource(xd200010Set.FILES) ;
this.dgListSub03.DataBind();
}
if (xd200010Set.XD300010S != null &&
xd200010Set.XD300010S.Length > 0)
{
this.dgListSub09.DataSource = CreateDataSource(xd200010Set.XD300010S) ;
this.dgListSub09.DataBind();
}
//???? ?
if (this.xd200010Set.XD930010S != null)
{
this.dgListSub11.DataSource = CreateDataSource(this.xd200010Set.XD930010S);
this.dgListSub11.DataBind();
}
//??? ?? ?? ??
if (xd200010Set.XD990010S != null &&
xd200010Set.XD990010S.Length > 0)
{
this.hlADDLINK.Visible = false;
this.dlLINK.DataSource = CreateDataSource(xd200010Set.XD990010S);
this.dlLINK.DataBind();
}
else
{
WS950010.XD950010 xd950010 = null;
xd950010 = wsTableID.GetTableID("TB200010", this.QUERYUSER);
this.hlADDLINK.Text = "??? ?? ?? ??";
this.hlADDLINK.NavigateUrl = "../UI900/UI990070.aspx?TABLEID="+ xd950010.TABLE_ID ;
}
I should work with tons of code like THAT. Fantastic, isn't it?
|
|
|
|
|
Something about that reminds me of Cobol...
|
|
|
|
|
This is a nice convention I think I should use it on my next project 
|
|
|
|
|
I don't think your company employs humans. Get out before they notice you're not one of them! 
- S
50 cups of coffee and you know it's on!
|
|
|
|
|
And run away as fast as he can, I might add.
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
|
|
|
|
|
Pssst.. I heard that they are soon running out of the 6 digits names, and going to migrate them to GUID...
|
|
|
|
|
darkelv wrote: Pssst.. I heard that they are soon running out of the 6 digits names, and going to migrate them to GUID...
ROTFLMAO
|
|
|
|
|
At least it's consistent. That's the idea, isn't it??
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
