how to reduce the code lines (1 Viewer)

amir0914

Registered User.
Local time
Today, 10:56
Joined
May 21, 2018
Messages
151
Hi all, I write the code to filter subform by two criteria on after update of textbox, but i think that's long and should be a way to brief that, Can someone help me or give a correct way to do this??

Code:
Private Sub Text5_AfterUpdate()

If IsNull(Me.Text5) And IsNull(Me.Text25) Then

Me.child85.Form.RecordSource = "SELECT * FROM qry_Customers2 "
 
ElseIf Not IsNull(Me.Text25) And Not IsNull(Me.Text5) Then

Me.child85.Form.Filter = "[S_Active] = '" & Me.Text5 & "'  And [fgk]=" & Me.Text25
Me.child85.Form.FilterOn = True
 
ElseIf IsNull(Me.Text25) And Not IsNull(Me.Text5) Then

Me.child85.Form.Filter = "[S_Active] = '" & Me.Text5 & "'"
Me.child85.Form.FilterOn = True
 
ElseIf Not IsNull(Me.Text25) And IsNull(Me.Text5) Then

Me.child85.Form.Filter = "[fgk]=" & Me.Text25
Me.child85.Form.FilterOn = True

End If
 

Gasman

Enthusiastic Amateur
Local time
Today, 17:56
Joined
Sep 21, 2011
Messages
14,045
Not sure why the record source is going to change, but....
Code:
Private Sub Text5_AfterUpdate()
Dim strFilter As String

Me.child85.Form.RecordSource = "SELECT * FROM qry_Customers2 "

If Not IsNull(Me.Text25) Then
    strFilter = "[fgk]=" & Me.Text25
End If
If Not IsNull(Me.Text5) Then
    strFilter = strFilter & " AND [S_Active] = '" & Me.Text5 & "'"
End If

If Left(strFilter, 4) = " AND" Then
    strFilter = Mid(strFilter, 5)
End If

If strFilter <> "" Then
    Me.child85.Form.Filter = strFilter
    Me.child85.Form.FilterOn = True
End If
End Sub

Not really shorter, but easier to understand perhaps?
HTH
 

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
What you had wasn't actually wrong (I am sure it worked) and the revision isn't really shorter but it does eliminate repetition of filter commands by use of a variable.
 

CJ_London

Super Moderator
Staff member
Local time
Today, 17:56
Joined
Feb 19, 2013
Messages
16,553
not sure it makes for good sql

but for shortest code

Code:
Private Sub Text5_AfterUpdate()

'why is this line required
Me.child85.Form.RecordSource = "SELECT * FROM qry_Customers2 "

Me.child85.Form.Filter = "[fgk]=" & nz(Me.Text25,"[fgk]") & " AND [S_Active] = '" & nz(Me.Text5,"[S_Active]") & "'"
Me.child85.Form.FilterOn = True
End Sub
 

jdraw

Super Moderator
Staff member
Local time
Today, 13:56
Joined
Jan 23, 2006
Messages
15,364
Amir0914,
For your own code readability and for anyone who may maintain the code, better to assign meaningful names to your controls. Child85 and text5, text25 don't convey any application meaning or usage. It doen't affect your question nor the response, just an observation on style for consideration.
Good luck.
 

sonic8

AWF VIP
Local time
Today, 18:56
Joined
Oct 27, 2015
Messages
998
Code:
Me.child85.Form.Filter = "[fgk]=" & nz(Me.Text25,"[fgk]") & " AND [S_Active] = '" & nz(Me.Text5,"[S_Active]") & "'"
I think using the column names as fallback for no filter is not very intuitive.

However, the real problem is that it's not going to work this way. The text delimiters must not be used around the [S_Active] column name. Sure, you can put that right but I guess, I would dislike the resulting code even more.

Unless setting the Filter property is required for whatever purpose, I would go probably for something like the following.

Code:
Dim WhereCondition as String
    
WhereCondition = WhereCondition & IIf(IsNull(Me.Text25.Value),"", " AND [fgk]=" & Me.Text25.Value)
WhereCondition = WhereCondition & IIf(IsNull(Me.Text5.Value),"", " AND [S_Active] = '" & Me.Text5.Value & "'")
        
Me.child85.Form.RecordSource = "SELECT * FROM qry_Customers2 WHERE 1 = 1 " & WhereCondition
 
Last edited:

The_Doc_Man

Immoderate Moderator
Staff member
Local time
Today, 12:56
Joined
Feb 28, 2001
Messages
26,999
First, brevity is often the least of your worries. You should always shoot for clarity because you will at some point probably need to revisit code, or someone else will. If your code is clear and straight-forward, no biggie. But if it looks like a plate of spaghetti, that IS a big deal.

Next... convert everything to meaningful names.

Finally, if that much typing gets to you, consider a WITH ... END WITH block so that you could do something like this:

Code:
WITH Me.Child85.Form

...

    .Filter=...
    .FilterOn=...

...

END WITH

That way, you shorten what you type and make the logic stand out more. You would still need the references to "Me.Textnn" inside the with-block since they have a different prefix, but all that diddling with the properties of the indicated subform could be shortened by three elements for each property you touch.
 

Cronk

Registered User.
Local time
Tomorrow, 04:56
Joined
Jul 4, 2013
Messages
2,770
I'll throw another option into the mix, embedded If statements rather than a series of ElseIfs


Code:
if IsNull(Me.Text5) then
  if IsNull(Me.Text25) Then
     ...
  else
    ...
  endif
else
  if IsNull(Me.Text25) Then
     ...
  else
    ...
  endif
endif
 

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
Another 2 cents worth:
Code:
With Me
.child85.Form.Filter = Nz("[fgk]=" + .Text25 & IIf(Not IsNull(.Text25), " AND ", "") + "[S_Active]='" + .Text5 + "'", "")
.child85.Form.FilterOn = True
End With
 
Last edited:

CJ_London

Super Moderator
Staff member
Local time
Today, 17:56
Joined
Feb 19, 2013
Messages
16,553
can be taken further

Code:
With Me.child85.Form
    .Filter = Nz("[fgk]=" + .Text25 & IIf(Not IsNull(.Text25), " AND ", "") + "[S_Active]='" + .Text5 + "'", "")
    .FilterOn = True
End With
 

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
No, I think Text5 and Text25 are on main form.
 

CJ_London

Super Moderator
Staff member
Local time
Today, 17:56
Joined
Feb 19, 2013
Messages
16,553
oh yes - just requires removing the . for those two controls
 

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
Sure, just won't get benefit of intellisense popup tips.
 

sonic8

AWF VIP
Local time
Today, 18:56
Joined
Oct 27, 2015
Messages
998
Sure, just won't get benefit of intellisense popup tips.
...and the expliciticity (is this a word?) of using a a explicit reference.

After looking through the suggestions made in this thread, including mine, I think most of them are driven too much by the intent to reduce the lines to a feasible minimum. - This is not really a sensible goal!

Sure, we should remove duplication from the code, but beyond that I would rather focus on the readability and how well the code conveys its intent. - I feel, several of the very brief alternatives of the code don't do very well in that regard.
 
Last edited:

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
Which was the intent of providing examples. I didn't say shortest was best, but OP asked so we showed. Now they are educated and can make a choice. Would I use my expression? Yes, I would.
 
Last edited:

The_Doc_Man

Immoderate Moderator
Staff member
Local time
Today, 12:56
Joined
Feb 28, 2001
Messages
26,999
If you look, Sonic8, my first sentence was about how you want clarity, not brevity, even though I had a suggestion for reducing typing.
 

amir0914

Registered User.
Local time
Today, 10:56
Joined
May 21, 2018
Messages
151
Hi to all guys for guide and advice me, I sincerely apologise for replying late to Response,

CJ_London, I use this code becuase subform nothing shows when the textboxes are empty

jdraw, You're right, I should to used meaningful names to controls, thanks so much for advice
sonic8, that's great, your code works fine.

June7, your code works very well but it nothing shows when textboxes are empty.

thanks to everybody for replying and advice and sorry for my poor english.
 

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
Works for me. Provide your exact code for analysis. Textboxes must be Null if empty, not have an empty string. If you want to provide db, follow instructions at bottom of my post.
 

amir0914

Registered User.
Local time
Today, 10:56
Joined
May 21, 2018
Messages
151
Works for me. Provide your exact code for analysis. Textboxes must be Null if empty, not have an empty string. If you want to provide db, follow instructions at bottom of my post.

Hi June7, sorry, your code works fine, I was wrong.
But I have another question, Can you add another field to the code?? my third field is "S_Type" and its textbox is "Text13"

it means the subform filters by 3 fields (text25,text5,text13).
 

June7

AWF VIP
Local time
Today, 09:56
Joined
Mar 9, 2014
Messages
5,423
That starts to get complicated because now the second AND operator is dependent on content of more than 1 control. But I just remembered a simpler syntax.

"[fgk] LIKE " & Nz(.Text25, "'*'") & " AND [S_Active] LIKE '" & Nz(.Text5, "*") & "' AND [S_Type] LIKE '" & Nz(.Text13, "*") & "'"

The first approach can be useful for concatenating fields such as name parts and want to avoid extra spaces or comma when a name part, such as middle name, is not present.
 

Users who are viewing this thread

Top Bottom