[ajross]

You seriously have trouble reading "i", "j", "param" and "num"? Hell, "ok", "depth" and "retry" are already in your English-language dictionary!

I'll grant that having variables named with "tmp" is confusing out of context, I guess. But if you're trying to start a Java-style war over this stuff, just recognize that most of the world has moved on and views names like those as perfectly fine when used within standard idioms.

[judemelancon]

I have trouble understanding what they're meant to signify, yes. I am a regular human with finite intellect; I admit it.

To actually comprehend this function requires storing those 14 names in short-term memory, reading through the over 300 lines of remaining code, filling in bits of the meaning of those names as they become clear, and only then reading the code again with that mental map. That's the case where none of the 14 have slipped my mind by the time I get back around. That just seems like an awful lot of overhead to net something that could be as easy as reading names if they were better chosen.

Just as a demonstration, why don't you time how long it takes you to figure out what j actually is and then report back?

[ams6110]

I don't know the codebase, but if for example, there's a project wide standard or convention for what the variable j means, then it could be OK. Or at least not as bad as it first appears. Not all code has to be written for an assumed stranger on the street trying to sight-read it.

[alricb]

This is OpenSSL we're talking aboot here.

[ectoplasm]

The use of shorthand names like that is a strong indication that the variables don't need to be live for ~319 lines. If you reduce the live range of your variables, you reduce the complexity of your function. Less complex functions are less likely to have bugs and are easier to diagnose when they do.

[jfoster]

Good points, but if you are able to do that, doesn't it suggest the function could easily be split into multiple, more specific functions?

[ectoplasm]

Yes, of course, it's just the first step. In fact you'd probably need to introduce a couple more variables. For example, i gets defined three times:

  i = sk_X509_num(ctx->chain);
  i = check_trust(ctx);
  i = X509_chain_check_suiteb(&ctx->error_depth, NULL, ctx->chain, ctx->param->flags);

What a mess. I would probably start by moving towards a module / class for all the functions that take either an X509_STORE_CTX *ctx pointer or something accessed through ctx.

[d_theorist]

Are you kidding me? There is no possible universe in which 'param' is an acceptable name for an argument.

[mayanksinghal]

What about this?

  class Function<T> {
    public apply(T param);
  }

[d_theorist]

OK. One possible universe.

[ectoplasm]

Technically speaking, that's a parameter, not an argument.

[Sanddancer]

A three hundred nineteen line function though with all those variables is more than a bit big. Modern compilers are rather good at inlining code, so it's doubtful there will be very much, if any effect on codesize there. The multitude of temporary variables also suggests that there are probably portions of that function that need factored out. OpenSSL suffers from a severe thousand paper cuts problem. While its warts aren't that bad when viewed individually, the sheer multitude shows a severe lack of organization of the project from top to bottom.