On ACF2023, we're regularly seeing the dreaded 'Branch target offset too large for short' error. My understanding is that there's too many conditionals in the cffunction for the JVM to handle. Our function is large, but isn't 'crazy' large, so this seems like a bit of a cop out. Is there something that can be done to stop this from happening (a magic setting somewhere?). We can't trim down the conditionals any more than we have, and cfincluding in a cfc feels like a ugly hack, but seems like our only option. Any suggestions? TIA

That's an underlying error from the JVM and you should never run into that with well-structured code, IMO. If your function is getting that error it is too large and needs to be refactored. There's no workaround: fix your code.

It's actually not bad code, subjective I guess. It's a big SQL statement in the datalayer that has many conditionals. Breaking it into multiple functions would be a nightmare to maintain / understand. Having it all there makes the most sense. Anyway, thanks for confirming there's no other way.

I've hit this before working with legacy code which people have just added to over the years and you just have to tackle it. You say We can't trim down the conditionals any more than we have why can't you move conditionals into functions? Can you give a example or code you can't refactor?

Yep

Sounds like using an SQL builder would be the solution - do you have tests?

Yeah nah, been down that road before. So if I simply CFINCLUDE the entire thing in a CFC function my problems disappear? Seems nuts if that's the solution others are suggesting on stackoverflow.

I've never tried using cfinclude to solve it - I've always refactored so don't know if that solves it.

Yes lots of ifs. Mainly to reduce the amount of data the query returns in different conditions. Improves performance. Think a getProducts() on a big ecom store. Lots and lots of different scenarios.

That conditional can be pulled out into it's own function though - and by putting into a function you can describe what it is doing.

Like #includePrices()# Where includePrices() returns a SQL statement text to get the prices?

No

<cfif includePrices(x,y)>pricescolumn,</cfif>

Mmm most of our conditionals are boolean already, so re-writing as a function probably won't win us much I'm guessing. Very little or few are multiple statements

You can even go further and have

includePrices

in it's own CFC so that the logic is separate (and therefore testable and reusable)

Yep I hear you on that. But does start to break down the readability. In my opinion. You now have to work your way through a load of functions to edit a SQL statement.

It's a worthwhile refactor anyway to move the logic and will help make the code readable, testable and will also reduce it - may not get you all the way though I appreciate that. An SQL builder is really the right solution but scary to refactor that with zero tests

There's no 'reuse' of these functions. The only reason they are there is to deal with this limitation! If the statements were used in multiple queries, sure, there's value.

The only reason they are there is to deal with this limitation!

Disagree - it makes you code testable - and as professionals we should all be writing tests. Ask the crowdstrike people about testing :D

Just to pitch in: The function is doing too much as you describe it. It should either build the SQL statement or run the SQL statement, not both. And the building-the-SQL part could perhaps use a Template Method approach where one has something like

buildTheStateemnt() {
    buildTheSelect()
    buildTheFrom()
    // etc
}

Refactoring (and keeping one's methods small and doing one thing) is not about reusability btw. it's clarity and testability.

I disagree. Feels super ugly that way. I don't think anyone would start out that way. SELECT id, name FROM sometable WHERE something = 1 Would never be written buildTheStateemnt() { buildTheSelect() buildTheFrom() buildTheWhere() } If you do that, man, you're nuts and good luck to you!

SELECT id, name
FROM sometable
WHERE something = 1

Would never be written

buildTheStateemnt() {
    buildTheSelect()
    buildTheFrom()
    buildTheWhere()
}

False equivalence. No-one's talking about refactoring a 50-byte SQL statement to use the template pattern. It's cos you've let it get away from you AND THE CODE NOW DOESN'T EVEN COMPILE (FFS!!!) that you are needing to ask how to mitigate it.

In the past at least, you couldn't use cfqueryparam anywhere except between cfquery tags. Not even in an included or cfmodule'd file that was only used between cfquery tags. Has that constraint been removed? It always annoyed me greatly, because it forced you into giant monolithic code.

Interesting. CF used to allow this:

<cfquery>
    <cfinclude><!--- params in here --->
</cfquery>

We had a lot of that back when I used to write code like Dave's (at the same company as Dave, even... hence him knowing I'm, nuts). Haven't tried anything like that for 15yrs or so though. But one only needs

<cfqueryparam>

if one still uses

<cfquery>

. Not needed with

queryExecute

(or even with the Query.cfc thing, from memory). It's a bit of an old-school approach to things.

Interesting, noted, works in CF2021 too. And yes old school.

I don't have anything else to add that hasn't been succinctly and correctly stated by Adam, Sean, et al. I will say I enjoy these kinds of architectural discussions. I may have to unleash my buddy in here soon. He's a principal architect level C# dev who I convinced to learn CF. In 6 days he's writing absolutely incredible code in CF, just beautiful stuff. He would eat this discussion up. 😄

You'd be SO famous. 😄

me and my buddy weird al

Yes, everyone is talking about Al. He's having a revival!

I write a lot of SQL logic with optional CTEs, optional inner/outer joins & subqueries using cfscript. Normally this would be a nightmare to write natively, but I used Ortus QB with my ACF app (without CommandBox, Forgebox or ColdBox) and, after working with it for a bit, found it to be easier to troubleshoot & unit test. QB (Query Builder) enables you to build a simple or complex SQL queries and then allow you to output or execute it. https://github.com/coldbox-modules/qb NOTE: If you want to use QB with any deprecated versions of ACF (ie, 2016 or 11), you'll need to download the v8.10.0 version.

Yup, query builders are a great way to go for anything involving conditionals. I work in Clojure these days and there's a library called HoneySQL that we use heavily at work for this (and I've ended up as the maintainer of it, since we rely on it so much).

Thanks @gamesover, very helpful.