firasdib / Regex101

This repository is currently only used for issue tracking for www.regex101.com
3.27k stars 199 forks source link

suggestions for a few code generator improvements (const, closure, lint, etc) #791

Closed cauerego closed 7 years ago

cauerego commented 7 years ago

please, take those suggestions as you will.

so, instead of this:

const regex = /wh(at)/g;
const str = `whatever`;
const subst = `-> \$1\\n--> `;

// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);

console.log('Substitution result: ', result);

it would look more something like this, which is also completely jslint compatible, as of today (although I'd bet you probably want to remove strict there, which would be fine):

// This creates a namespace scope
var myRegExp = (function () {

    // And this is yet another good javascript practice
    "use strict";

    // Now let's define some variables
    var regex = /wh(at)/g;
    var str = "whatever";
    var subst = "-> $1\n--> ";

    // The substituted value will be contained in the result variable
    var result = str.replace(regex, subst);

    // Print out the result
    console.log("Substitution result:", result);
}());
firasdib commented 7 years ago

The code is completely ES6 compatible. Is there any reason you want to jump back to ES5?

cauerego commented 7 years ago

nope. I just don't care for ES6 yet. since it's not widespread. haven't even bothered to learn more about it. I guess you'd replace var for let in that case, not sure what else.

firasdib commented 7 years ago

You should give it a go, I don't know of anyone who still willingly uses ES5 :-)

cauerego commented 7 years ago

well, try jslint.com for starters. they use ES5. and android. http://kangax.github.io/compat-table/es6/

TWiStErRob commented 7 years ago

since it's not widespread

All major browsers (except "IE" of course) fully support it 2-3 versions back... and if you use the stock browser on Android, bless your soul! ;)

I also replaced quote character with jslint standard

jslint is not a bible, it's a guideline, and also configurable

In this code generator's case it makes more sense to use the template quotes because the input and the replacement could be multi-line, which would require a lot of escaping and concatenations without the backticks.

cauerego commented 7 years ago

IE is not a browser! I think it never was. and of course I agree with jslint. I actually would say same thing about the bible: it's a guideline and configurable.

I've been reading about ES6 since your first suggestion, though... I can now see why most of it is there, including the escapes. but, since this turned into a "let's teach me some js updated common sense":

I used to prefer single quotes, just because HTML standard is double quotes and so functionally they make more sense. but I came to quickly agree with jslint in this one. for most of the code, and especially now js is not co-dependent of HTML, double quotes make much more sense, just thanks to easier readability alone.

thanks again for the quick class! ;)

firasdib commented 7 years ago

why const? and;

Are you planning on reassigning the variable? If you did reassign it, would the code still function as you intended? Its just how I like to code, you don't have to agree with it.

why single quotes (on the console.log)?

Again, my preference :-)

TWiStErRob commented 7 years ago

Agree with const, if the keyword is, let's use it as intended. They're literal constants anyway; and in case of result the value should still be untouched and only read.


Here's how I use quotes when I write JS:

Reasoning: since we have two quotes, we can make use of them. The single quotes for programming came from the old days when we generated HTML code via string concatenation: it was easier to read and write 'src="' + src + '"' than "src=\"" + src + "\"" Another potential pro: ' requires a single keystroke on US and UK keyboard, " needs a SHIFT

cauerego commented 7 years ago

plan to reassign? yes. code still function as intended? yes. as long as the variables are reassigned all at once. but at least I see your point now... I think both (const and var or let) are defunct in such an out of context small snippet. ideally we'd put it in a function or something but then it starts to get much bigger than needed... and even just wrapping it with a function as it is would also make no descriptive sense as you brought up.

maybe my main point with not using const, though, is that just by testing it on browser console sucks - you can only run it once. and there's no practical benefit in such a small code to say they're const other than that valid point of view.

for that shift argument: 🤣

I actually like your quote reasoning @TWiStErRob... but now I wonder if we shouldn't adopt the backtick quote. that one probably makes the most technical sense and stylistic as well, from the little I've been reading.

TWiStErRob commented 7 years ago

Re: console testing that's actually a good point, but I would assume you only generate code once your regex works, so why would you need to run it multiple times?

So far having a third type of quote has been confusing, but it has its uses: multiline constants, and interpolation: `src="${src}"`

I think the current generator shows a lot of good practices. The practical benefit of having such code (e.g. consts) is to propagate good practices so that when they "re-use" this code in production it's better. Seeing a const var immediately puts it into context, and you can be sure it won't be changed. One less scoping rule to worry about understanding the code.

If we do the function wrapping it may confuse total newbies, asking the question: "How do I get the result out of that wrapped thingie?"

cauerego commented 7 years ago

same reason you run it multiple times under regex101: testing. the console is all about debugging, after all. I suppose :P it's simple enough to wrap it with curlies { } and it works fine in console. just replacing const with let wouldn't work anyway.

on a side note, as soon as I started writing in ES6, got into a compatibility issue: can't find a reliable minifier. anyhow, back to const, and another small note for my future self...

I just went to a modern art exposition, which I hate because it's so meta. it's art made for artists, and pretty much meaningless if you don't have the context - which the excruciating majority of people don't. maybe I still didn't quite get the point on that const there now as I'd wish and hope I do, but I still get this same feeling about it. perhaps it's because of the too-generic names... or lack of closure and then it looks like a global const. it could as well be just me and my brain farting.

it's probably better to just nevermind me! :D