[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 430
  • Last Modified:

COnfusing code

Greetings experts:

The code below is so confusing. It is one of the codes by a former colleague that I am responsible for now. I will like to rewrite it but it is so confusing, I am not sure what to do.

Your expert assistance with rewriting this code is truly, truly appreciated.

Many thanks in advance
This is the actual coe below:


ConditionsElementValues = ""
	sqlstr	= "Select E.VName,E.ConditionsGroup from ConditionsElement E, ConditionsGroup G where E.ConditionsGroup = G.VName and E.active=1 order by E.ConditionsGroup"
    objRs.Open sqlstr, objconn
    cnt = 1
    arrcnt = 1
    if not objRs.eof then
		do while not objRs.EOF
			if CondElementNew	<> objRs("ConditionsGroup") then
				arrcnt = 1
				GenJSCondElementArr		= GenJSCondElementArr & "SubCat" & cnt & " = new Array()" & chr(13)
				CondElementNew	= objRs("ConditionsGroup")
				GenJSCondElementArrVal	= GenJSCondElementArrVal & "SubCat" & cnt & "[" & arrcnt & "] = '" & objRs("VName") & "'" & chr(13)
				cnt = cnt + 1
			else
				GenJSCondElementArrVal	= GenJSCondElementArrVal & "SubCat" & cnt - 1  & "[" & arrcnt + 1 & "] = '" & objRs("VName") & "'" & chr(13)
				arrcnt = arrcnt + 1
			end if
			
			LookUpDataValue		= objRs("VName")
			ConditionsElementValues = ConditionsElementValues & "<option value='" & LookUpDataValue & "'>" & LookUpDataValue & "</option>"
			objRs.MoveNext
		loop
	end if
	objRs.Close 
	ConditionsElement 	= "<select name=ConditionsElement class=reqfields style=""width:90px; height:22px;FONT-SIZE: 8pt""><option value=>&nbsp;</option></select>"

Open in new window

0
sammySeltzer
Asked:
sammySeltzer
  • 25
  • 19
  • 14
1 Solution
 
BadotzCommented:
Does the code work?
If so, why change it? It may *seem* confusing, but that may just be inexperience.
If not, then you had better understand why before attempting to "fix" it.
0
 
sammySeltzerAuthor Commented:
it doesn't work and i wanted it rewritten so it is easy to maintenance.
Since you understand it sir with your wealth of experience and expertise, could you please help?
Thank you
0
 
BadotzCommented:
What is is supposed to do?
0
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
sammySeltzerAuthor Commented:
Just looking at the screen, it is supposed to be populated based on selection from another dropdown.

The reason I didn't post the code for the other dropdown is because I understand how that works and can rewrite that easily.

So, you select an option from the dropdown from that other code, and it populates this one with a list of values based on your selection from that other dropdown but sometimes, the value from this dropdown is blank even though there are values in the DB.

Thanks so very much for your assistance
0
 
BadotzCommented:
While "ConditionsElementValues" may contain "<option>" values, it is never stuffed into the "<select>" at the end of your code.

I would change this:

ConditionsElement = "<select name=ConditionsElement class=reqfields style=""width:90px; height:22px;FONT-SIZE: 8pt""><option value=> </option></select>"

to this:

ConditionsElement = "<select name=ConditionsElement class=reqfields style=""width:90px; height:22px;FONT-SIZE: 8pt"">" & ConditionsElementValues & "</select>"
0
 
BadotzCommented:
BTW: There is nothing confusing about this code. It is as straightforward as it gets.

It may be difficult to read because of the lengthy variable names, but it is not confusing.
0
 
BadotzCommented:
I made a few changes, most of which are cosmetic. I do not rely on default values, but that is just a personal preference.
ConditionsElementValues = ""
 
sqlstr = "Select E.VName,E.ConditionsGroup " & _
"FROM ConditionsElement E, ConditionsGroup G " & _
"WHERE E.ConditionsGroup = G.VName AND E.active=1 " & _
"ORDER BY E.ConditionsGroup"
 
objRs.Open sqlstr, objconn
 
cnt = 1
arrcnt = 1
do while not objRs.EOF
    if CondElementNew <> objRs("ConditionsGroup").Value then
        arrcnt = 1
        GenJSCondElementArr = GenJSCondElementArr & "SubCat" & cnt & " = new Array()" & chr(13)
        CondElementNew = objRs("ConditionsGroup").Value
        GenJSCondElementArrVal = GenJSCondElementArrVal & "SubCat" & cnt & "[" & arrcnt & "] = '" & objRs("VName") & "'" & chr(13)
        cnt = cnt + 1
    else
        GenJSCondElementArrVal = GenJSCondElementArrVal & "SubCat" & cnt - 1  & "[" & arrcnt + 1 & "] = '" & objRs("VName") & "'" & chr(13)
        arrcnt = arrcnt + 1
    end if
    
    ConditionsElementValues = ConditionsElementValues & "<option value='" & objRs("VName").Value & "'>" & LookUpDataValue & "</option>"
    objRs.MoveNext
loop
 
objRs.Close 
ConditionsElement = "<select name=ConditionsElement class=reqfields style=""width:90px; height:22px;FONT-SIZE: 8pt"">ConditionsElementValues</select>"

Open in new window

0
 
sammySeltzerAuthor Commented:
well,that's your experience speaking which is nice to have but I still think this code can be written with fewer lines of code.

I guess I need to keep playing with it till I can get it.
0
 
BadotzCommented:
Sorry, the last line should read:

ConditionsElement = "<select name=ConditionsElement class=reqfields style=""width:90px; height:22px;FONT-SIZE: 8pt"">" & ConditionsElementValues & "</select>"
0
 
BadotzCommented:
Have you tried any of my suggestions?

Why do you think the code is confusing?

Why do you think it can be rewritten using fewer lines of code? If it could be done, what would the point of that be?
0
 
sammySeltzerAuthor Commented:
Here is an example of what I meant.

The top code above is the code I indicated to you is like the category that you select and then sub cats associated with it populate the code you and I are working on now.

See the colleague's version and below it is my version.

You can see it is easier to understand and modify then the colleague's version, at least for me anyway.

I would like preferred something similar to the code you are helping out with.

Have a look below.


            sqlstr      = "Select VName from xlkpS_ConditionsGroup where active=1 order by VName"
               objRs.Open sqlstr, objconn
               cnt = 1
            if not objRs.eof then
            do while not objRs.eof
                  LookUpDataValue            = objRs("VName")
                  ConditionsGroupValues = ConditionsGroupValues & "<option nodevalue='" & cnt & "' value='" & LookUpDataValue & "'>" & LookUpDataValue & "</option>"
                  cnt = cnt + 1
                  objRs.MoveNext
            loop
            end if
            objRs.close
            ConditionsGroup       = "<select class=reqfields name=ConditionsGroup style=""height:20px;FONT-SIZE: 8pt"" onchange=FillCondElement()><option value=>&nbsp;</option>"      & ConditionsGroupValues      & "</select>"

               <td><<%=ConditionsGroup%></td>





               <td>
                 <%
              SQL      = "Select VName from xlkpS_ConditionsGroup where active=1 order by VName"
              set CGroupSet =  conn.Execute(SQL)
              %>
                  <select name=ConditionsGroup style="background-color:#ffff90;FONT-SIZE: 8pt" onchange=FillCondElement()>
              <option value="">&nbsp;</option>
              <%
              Do Until CGroupSet.EOF
              CGroupID = CGroupSet("VName")
              If CGroupID = ConditionsGroup Then sel = "SELECTED" Else sel = ""
               %>
              <OPTION Value="<%=CGroupID%>" <%=sel%> > <%=CGroupSet("VName")%>
              <%
              CGroupSet.MoveNext
              Loop
              %>
              </SELECT>
             </td>
0
 
BadotzCommented:
I will do nothing further until you have answered all of my other questions, and have tried the code I supplied.
0
 
sammySeltzerAuthor Commented:
Ok, fair enough.

Have you tried any of my suggestions?
Yes,  It is giving me the same result whereby the values are there on the db but it shows blank.

The changes you made apparently didn't fix it.

Why do you think the code is confusing?
Because I don't understand why the following are used:


    cnt = 1
    arrcnt = 1
    if not objRs.eof then
            do while not objRs.EOF
                  if CondElementNew      <> objRs("ConditionsGroup") then
                        arrcnt = 1
                        GenJSCondElementArr            = GenJSCondElementArr & "SubCat" & cnt & " = new Array()" & chr(13)
                        CondElementNew      = objRs("ConditionsGroup")
                        GenJSCondElementArrVal      = GenJSCondElementArrVal & "SubCat" & cnt & "[" & arrcnt & "] = '" & objRs("VName") & "'" & chr(13)
                        cnt = cnt + 1
                  else
                        GenJSCondElementArrVal      = GenJSCondElementArrVal & "SubCat" & cnt - 1  & "[" & arrcnt + 1 & "] = '" & objRs("VName") & "'" & chr(13)
                        arrcnt = arrcnt + 1
                  end if
                  
                  LookUpDataValue            = objRs("VName")
                  ConditionsElementValues = ConditionsElementValues & "<option value='" & LookUpDataValue & "'>" & LookUpDataValue & "</option>"
                  objRs.MoveNext
            loop
      end if

and notice that arrCnt was used twice.

and what is this doing:

GenJSCondElementArr & "SubCat" & cnt & " = new Array()" & chr(13)

it is also used twice.

Please understand that if I understand what the entire code is doing, i will be asking a different question.

MY goal is to write a code that is easy to explain and modify.

The code I posted last is easy to understand and modify. This is not to me.

Might be to you and that's why I came to experts like you to see if you could come up with a completely different approach. I will increase points if need be.

Sorry for inconvenience and thanks for your assistance.
0
 
BadotzCommented:
>>and notice that arrCnt was used twice.

[arrCnt] counts the times [if CondElementNew <> objRs("ConditionsGroup")] evaluates to FALSE. It is *used* twice, but it may be *incremented* more than once within the LOOP.

>>GenJSCondElementArr & "SubCat" & cnt & " = new Array()" & chr(13)

It appears to be inserting JavaScript into [GenJSCondElementArr]. Note the "JS" in the variable name.

Anything else? Oh, right - it doesn't work.

Has it ever worked?
0
 
sammySeltzerAuthor Commented:
I said this is not my code.

It was written by a colleague who is no longer with us.
0
 
BadotzCommented:
>>I said this is not my code.

When did I ever ask? Please answer my questions; I do not ask them idly.
0
 
BadotzCommented:
And it *IS* your code now.
0
 
sammySeltzerAuthor Commented:
You are right now,  Badotz - it is my code and I need your help to make it more efficient than it is.

Efficient in that I believe there are easier and simpler ways to write that code to produce the same result, even though I am having problem doing it myself.

So, to answer your question, I honestly don't know.

All I know is that the users brought to my attention the fact that the code isn't working as intended among other list of enhancements they wanted done.

Thanks a lot for your continued assistance.
0
 
BadotzCommented:
From what I can see, there is no reason for the code I supplied not to work.

This, from your larger example, is wrong:

<td><<%=ConditionsGroup%></td>

It should be:

<td><%=ConditionsGroup%></td>

As for efficiency, well, that is a two-edged sword. How much time are you willing to spend to make this smattering of code "as good as it can be"? And if you re-visit it a year from now, will it still be "as good as it can be"?
0
 
sammySeltzerAuthor Commented:
that was the colleague's code is *not* being used anymore.
0
 
BadotzCommented:
>>See the colleague's version and below it is my version.

That is from where the code was copied, the code below your "colleague's" code. How is it not your code?

But that is beside the point. I'm afraid that there is little more I can do for you. Without a clear view of the problem, and your unwillingness (or inability) to follow along, I am bowing out.

If you have a specific question, ask, and I will try to help. But I will not remain on this path to nowhere.
0
 
sammySeltzerAuthor Commented:
Thank you very much.

I think we both would have been better off with someone who understands what I am after and can assist with the solution.

I have provided you with all that is needed to help out but I can understand that ...
0
 
BadotzCommented:
I'm sorry you think that I don't understand what you are after. Truth is, you are right. I haven't a clue.

Perhaps you could enlighten me: What are you after?
0
 
Wayne BarronCommented:
Hello Sammy.
Are you wanting to have the following:

2 Dropdown Boxes. (Menus list/Menu,,, or what ever you want to call them)

When you select an item from the first menu, it populates the items to the 2nd menu to match?
0
 
Wayne BarronCommented:
Sammy.
What are you doing this this one?
0
 
sammySeltzerAuthor Commented:
hi carrzkiss:

Do you mean the technology or db?

If technology, then ASP; if db, then MS Access.

Thanks for asking.
0
 
Wayne BarronCommented:
I was basically asking if you was done with this one?
If not, then this is what I asked.
Let me know if this is what you are wanting to do?

==========================Previous Post 3 post up============
carrzkiss:
Hello Sammy.
Are you wanting to have the following:

2 Dropdown Boxes. (Menus list/Menu,,, or what ever you want to call them)

When you select an item from the first menu, it populates the items to the 2nd menu to match?
0
 
sammySeltzerAuthor Commented:
yes, basically.

thank you
0
 
Wayne BarronCommented:
can you provide me with a sample of your database please?
Just upload it to the site.
0
 
sammySeltzerAuthor Commented:
that's a huge db. Ok, please give me time to come up with something.

Thanks again
0
 
Wayne BarronCommented:
Duplicated your DB
Keep on a few rows of data in it.
And Delete the rest. Get the DB down to 1mb or less.

Do not send over a HUGE DB, that is to time consuming.

Carrzkiss
0
 
sammySeltzerAuthor Commented:
ok, here is a sample.

Thanks again. Please let me know when you grab it.
test.mdb
0
 
Wayne BarronCommented:
I've got it
0
 
Wayne BarronCommented:
We have 2 Tables here.
Does the [ConditionsElement] control the [ConditionGroup]?
Parent =  [ConditionsElement]
Child = [ConditionGroup]

As in:
You want the [ConditionsElement] to populate the 1st Menu
And you want the Resulting Items from the  [ConditionGroup] To populate the 2nd Menu.

Is this correct?
0
 
sammySeltzerAuthor Commented:
Yes, that is correct.

Thank you!
0
 
Wayne BarronCommented:
Also.
What fields do you want to Display in the Menu's
Please let me know which Fields for the Main Menu and which for the Child Menu.
0
 
sammySeltzerAuthor Commented:
Just leave blank.

For instance, let it be something like this:

<option value="">&nbsp;</option> and then let get the values from the db.

Please let me know if I miss understand your question.

0
 
Wayne BarronCommented:
This is pretty simple.
I am sure that you will understand it.
Please read the accompaning Readme.txt file for IMPORTANT information
Regarding your Database.
Please open up the database that is in the zip file, and look at how it is set up.
While reading the Readme.txt file. It will help you to understand it better.

http://ee.cffcs.com/Q_24133261/Q_24133261.asp
Code
http://ee.cffcs.com/Q_24133261/Q_24133261.zip

Have a good one.
Carrzkiss
0
 
sammySeltzerAuthor Commented:
Dont' hate me please. I think I told you wrong.

One of the tables has 6 values and that table is the conditionsGroup.

Those values are Residential, Safe House,Shelter,Medical, and Vehicle.

That is the category. You select any of those values and the associated values populate the second dropdown.

I am very, very sorry about the mistake and thank you for doing this very quickly. You are very good indeed.
0
 
Wayne BarronCommented:
Not a problem at all.
Replace all the code in the file with this.
And you will be set.

Carrzkiss
<%@LANGUAGE="VBSCRIPT" CODEPAGE="1252"%>
<%
     ' MapPath of virtual database file path to a physical path.
     ' If you want you could hard code a physical path here.
     strDBPath = Server.MapPath("test.mdb")
     ' Create an ADO Connection to connect to the sample database.
     ' We're using OLE DB but you could just as easily use ODBC or a DSN.
     Set conn = Server.CreateObject("ADODB.Connection")
     ' This line is for the Access sample database:
     conn.Open "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & strDBPath & ";"
%>
<script language = "JavaScript">
<!--
// Populate an array with all of the models from the different 
// manufactures in the order that the manufacturers appear in the 
// drop-down list. In this example, I'll use Ford and Toyota
// of course, you'll want to generate these via server-side vbscript
 
subcat = new Array()
subcat [0] = new Array(1)
subcat [0][0] = " "
<%
catcount = 0
sqlstring = "SELECT * from ConditionsGroup"
set rs1 = conn.execute(sqlstring)       
do while not(rs1.eof)
catcount = catcount + 1
response.write "subcat ["& catcount &"] = new Array()" & vbcrlf
      sqlstring = "select * from ConditionsElement where RID = " & rs1("RID")
      set rs2 = conn.execute(sqlstring)
      subcatcount = 0
            Do while not(rs2.eof)
            response.write "subcat ["& catcount &"] ["& subcatcount &"] = """ & rs2("VName") & """" & vbcrlf
            subcatcount = subcatcount + 1
            rs2.movenext
            loop            
rs1.movenext
loop%>
 
//============================================
 
//Next, we create a function to fill the second drop down from 
//the array based on the item selected in the first drop down.
 
function FillList()
{
var num=document.formname.catagory.selectedIndex 
var boxlength = 0
 
document.formname.subcat.selectedIndex = 0
for ( ctr=0;ctr<subcat[num].length;ctr++)
 {
 boxlength++;
 document.formname.subcat.options[ctr] = new Option(subcat[num] [ctr], subcat[num][ctr]);
 }
document.formname.subcat.length = boxlength;
document.formname.subcat.options.length = boxlength;
document.formname.subcat.focus() ;
}
//-->
</script>
 
</HEAD>
<BODY>
<FORM action="" method=post id=form1 name=formname>
<%
cat = Request.QueryString("cat")
sqlstring = "SELECT DISTINCT * from ConditionsGroup"
set rs = conn.execute (sqlstring)
%>
 
<SELECT id=catagory name=catagory onChange = "JavaScript:FillList()">
<OPTION selected value=""></OPTION>
<%
do until rs.eof
%>
<OPTION value="<% = rs("VName")%>"><% = rs("VName")%></OPTION>
<%
rs.movenext
loop
set rs = nothing
%>
</SELECT>
 
<p>
<SELECT id=select2 name=subcat>
<OPTION></OPTION>
</SELECT>
 
</FORM>
</BODY>
</HTML>

Open in new window

0
 
Wayne BarronCommented:
0
 
sammySeltzerAuthor Commented:
Something is still not right.

For instance, according to the output I am given at work, if you selet Residential from the first fropdown, the second dropdown gets populated with the following:

Program Pays
No Pay
Client Pays
Prog/Client Pays
0
 
sammySeltzerAuthor Commented:
Sorry, I pushed the submit before I was done.

I was going to say that if you select Safe House, you get something similar.
if you select Shelter, you get No pay, etc but with your great work, we get different result.

any ideas?

I know it is me telling wrong...really sorry. too excited to have gotten help, I am making mistakes.
0
 
Wayne BarronCommented:
The database that you sent me over.
Its values are not correct.
(See image below)
So. unless I actually see your entire "CORRECT" datebase, It is going to be hard to direct you
In what the problem is.

The code how-ever is 100% on the money.
It is your database that is not. For what I am seeing.

Please see below image for what I am talking about.

Carrzkiss
Q-24133261.jpg
0
 
sammySeltzerAuthor Commented:
Please take a look at the conditionsElement table.

It has 2 important fieldnames = VNAME and COnditionsGroup.

Each value from the VNAME fieldname has one or more corresponding Values from the ConditionsGroup fieldname.

When we select say, Residential, it needs to pick those associated values from ConditionsGroup fieldname from the conditionsGroupElemet table, hope I am making sense.

Thanks a lot for the help

Perhaps the 2 fieldnames will need to be joined at first, no?
0
 
Wayne BarronCommented:
Your database is not setup properly.

You need some uniform on your database.
I am going to fix it, and send over a working structure for you.
It is going to be up to you to do the same thing to your real database.
If you feel that you cannot do it, then let me know and we will think of something together.

Carrzkiss

0
 
sammySeltzerAuthor Commented:
please go ahead and try your solution.

Thank you again
0
 
Wayne BarronCommented:
?
What is the deal with the VName in both tables?
They really have nothing to do with the JOIN.
You are connecting together

ConditionsGroup - VName
with
ConditionsElement - ConditionsGroup

Not with
ConditionsElement - VName

Help me to understand what the VName is going to do with the Menu's?
They cannot be added to the Menus, as they are there own Field Values.
0
 
sammySeltzerAuthor Commented:
yes, I agree with you. THis is someone' elses db and ap I am trying to simplify
0
 
Wayne BarronCommented:
Never mind.
I got it, I think.

take a look at it now.

http://ee.cffcs.com/Q_24133261/Q_24133261.asp
Code
http://ee.cffcs.com/Q_24133261/Q_24133261.zip

Look at your database within the .zip file.
You are looking at the: ConditionsGroup.RID       It is of TYPE  [AutoNumber]
And the ConditionsElement.RID . Number - List goes with the ConditionsGroup.RID

Carrzkiss
0
 
Wayne BarronCommented:
:) works like a charm, if I do say so myself.

You have a lot of work cut out for you.
You have to match your Fields RID #'s with one another in order for it all to work
Like it does in the demo.

Good Luck
And let me know if you need some help.

Carrzkiss
0
 
sammySeltzerAuthor Commented:
You got it!!!!!!!!!!!!!!!!!!!

Thank you

Great job!

You are indeed a guru.

Thank you so very much.

Let me play with it now.
0
 
Wayne BarronCommented:
on line: 68
sqlstring = "SELECT DISTINCT * from ConditionsGroup"
Remove the DISTINCT
Should look like this
sqlstring = "SELECT * from ConditionsGroup"

The DISTINCT is not needed in this version of the code.

Going to get some sleep now.
I hope that you can understand the code.
And it is nice to read the kind words that you have written.

Maybe we can get this post Finalized this evening, as it is WAY Old.
Have a good evening, And going to get some rest now.

2:10AM EST USA
Carrzkiss
0
 
sammySeltzerAuthor Commented:
Thank you, thank you, thank you.

I knew someone like you would come along.

Many thanks.
0
 
Wayne BarronCommented:
:)
You rock.
Keep it up!
It is pretty easy once you get the hang of it.
It took me many years to grasp a hold of it, and now I love every minute that I get to help
Someone make their coding dreams come true.
At times, I enjoy it more then I enjoy doing my own coding for my own stuff.
(Of which I am really behind on :) )

Going to bed now.
Have a good one Sammy.
Nice working with you again.

Carrzkiss
0
 
Wayne BarronCommented:
Congradulations: You have [0] open questions now :)

have a good one
Carrzkiss
0
 
sammySeltzerAuthor Commented:
Many, many, many thanks.

I hope you can be the one to help me next time I have another question, I am sure real soon too as I continue to clean up existing stuff.
Thanks alot and good night
0
 
sammySeltzerAuthor Commented:
I want to thank you again so very much for helping me get out of the mess that I have been in for so long.
You just don't know it felt like today to go to my boss and tell him that that task is now working.

Thanks alot.

Now, I have something similar but with a twist.

I have posted the question here:

http://www.experts-exchange.com/Web_Development/Web_Languages-Standards/ASP/Q_24250288.html

If your time permits, I hope you can help me again.

Hopefully, database won't be needed this time.
0

Featured Post

Keep up with what's happening at Experts Exchange!

Sign up to receive Decoded, a new monthly digest with product updates, feature release info, continuing education opportunities, and more.

  • 25
  • 19
  • 14
Tackle projects and never again get stuck behind a technical roadblock.
Join Now