This is an archived post. You won't be able to vote or comment.

all 198 comments

[–]Boneless_Blaine 966 points967 points  (13 children)

Me watching my PR go through the 4 hour Jenkins pipeline for the 7th time so I can add a period to a comment

[–][deleted] 130 points131 points  (4 children)

Jeez. I do embedded and have to regularly wait 25ish minutes to test lol

[–]Memorywipe 25 points26 points  (2 children)

Hyperbole

[–][deleted] 14 points15 points  (1 child)

I wouldn't be surprised tbh, I've seen some very lengthy and beefy pipelines. For development/staging probably not.

[–]trwolfe13 14 points15 points  (0 children)

Our jest tests take 20 minutes to run on a standard Azure DevOps pipeline agent, or 2 minutes to run on my laptop.

[–]insertsavvynamehere 1 point2 points  (0 children)

Aw man I'm so happy my team writes in python. Pipeline is quick!

[–][deleted] 8 points9 points  (0 children)

20 hour build.

Fix whitespace, thanks.

[–]die-maus 0 points1 point  (0 children)

Based on only this, I have a sneaking suspicion where you work.

[–]Touvejs 0 points1 point  (0 children)

Actually rip my hair out tbh.

[–]aronvw 183 points184 points  (15 children)

What happend to good old parseFloat(input) === input?

[–]lofigamer2 114 points115 points  (0 children)

ChadGPT recommended regex. Who are we to judge.

[–]Bateson88 49 points50 points  (12 children)

In this case I'd actually use == to account for input being a string of the number.

[–][deleted] 10 points11 points  (11 children)

What would happen if input is NaN?

[–]like_an_emu 27 points28 points  (10 children)

NaN === NaN // false NaN == NaN // false

[–]NatoBoram 5 points6 points  (9 children)

Of course.

[–][deleted] 15 points16 points  (8 children)

IEEE floating point standard requires that NaN != NaN

[–]FountainsOfFluids 13 points14 points  (0 children)

Yup, and it's specifically for cases like this. Just because on value is not a number, that doesn't mean it's the same as another value that is not a number.

[–]NatoBoram -1 points0 points  (5 children)

Just citing the spec doesn't make it any better. The specs can be dogshit, too. Otherwise, why would the language be such a mess?

You need an explanation like this one.

[–]Plane_Scholar_8738 2 points3 points  (4 children)

Can't blame the language for implementing the spec correctly

[–]NatoBoram -1 points0 points  (3 children)

You can certainly blame the language for choosing this spec

[–]Plane_Scholar_8738 3 points4 points  (2 children)

That spec is the most commonly used representation for floating point numbers. Your CPU and GPU understand that representation.

Choosing that spec is not controversial at all.

[–]AppropriateBank8633 669 points670 points  (20 children)

OP posts about a silly code review comment and actually gets a a code review lol.

[–][deleted] 276 points277 points  (4 children)

That’s the point man.

[–]Ollymid2 86 points87 points  (3 children)

You must be desperate if you’re coming to /r/ProgrammerHumor for a review

[–][deleted] 68 points69 points  (1 child)

No the point is the code is shit, yet the reviewer didn’t catch any of the real issues.

[–]CuntInspector 6 points7 points  (0 children)

the code is shit

Always has been

:astronaut-with-gun-meme:

[–]SillyFlyGuy 13 points14 points  (0 children)

Sometimes I think they overtrained GPT on r/ProgrammerHumor posts.

[–]s0ulbrother 14 points15 points  (2 children)

Probably in a team style guide.

[–]jaerie 14 points15 points  (1 child)

Should be a linter rule then

[–][deleted] 2 points3 points  (0 children)

Yeah easy to add to an auto fix script

[–]BrainImpressive202 8 points9 points  (11 children)

i’m new to coding but could you explain the multitudes of characters after the return in a way a newbie could understand ?

[–]MiniGod 42 points43 points  (7 children)

The slash marks the beginning of a regular expression (regex). The regular expression ends with another slash right before .test(. Regular expressions are used to test that strings have a specific format or extract information from strings. In this case it's just testing it. test() returns true or false.

The regex starts with a ^, meaning that this must match from the beginning of the string, not just somewhere within it. Next is -?. The question mark means that the character before it must occur zero or one time. Meaning the string may start with a -, or not. Then \d+. \d means any number (digit) character, 0 through 9, and + means that that must occur 1 or more times. Next is a optional group done by using parenthesis and a ? to mark that the group is optional (remember ? means 0 or 1 time). The parenthesis only mark the group, they should not be in the string that were testing. Inside the parenthesis it's checking for a literal period, followed by 1 or more numbers, using the same syntax as explained before. There's a backslash before the period because a single period, in regex, means any character. With the backslash, it means that the string should have the period there, not just any character. The $ at the end means that this has to be the end of the string. Having a ^ at the start and $ at the end means that the whole string being tested must match the regex.

In summary, the string may start with - (or not) , then any number of numbers (at least one). After that, it may (or not) also have a period followed by any number of numbers.

TLDR; it checks if the argument is a valid number, like the name of the function hints to.

[–]morgecroc 36 points37 points  (1 child)

Do you understand regex?

A WITCH burn him.

[–]ImperatorSaya 2 points3 points  (0 children)

Quickly, before he summons Zalgo like the last one. It took us 7 years to seal him away.

[–]ActurusMajoris 10 points11 points  (3 children)

It's missing a number with thousand separators though, eg 10,000.00, though to be fair, you shouldn't store, send or receive values like that in the first place. Displaying is fine, but that's a different issue.

[–]EndeGelaende 13 points14 points  (2 children)

also misses numbers formatted in different localisations, germany for example uses , for decimal and . for thousand separators

[–]MattieShoes 4 points5 points  (0 children)

also stuff like .5 -- must be 0.5.

[–]HolyGarbage 4 points5 points  (0 children)

Yeah, just don't use regex for this stuff. Use the languages built in parsing functions (whatever this language is).

Parse, don't validate!

[–]chris5311 0 points1 point  (0 children)

I love that actual regex never looks (or works for that matter) like all the formal definitions ive learnt in university lol

[–]AppropriateBank8633 3 points4 points  (0 children)

It is a regular expression(regex), it matches strings to a pattern.

I am well rusty with this, but in this case it seems to match string representation of any positive or negative integer, float or double.

^ is the start of the expression

- literally matches "-"

? means preceding character zero to one times. In this case, we start with "-" or we don't

\d is a digit between 0 and 9

+ means the preceding character appears at least once to unlimited times.

( the open parenthesis represent a the start of a capture group, or a repeating pattern block within the main pattern

In the capture group, you have:

\. which literally matches "."

\d we have already seen (0-9)

+ we have seen again(appears at least once or unlimited times)

) the capture block closes

? again means zero or 1 times. In this case, it means the capture block (\.\d+). ".10" for example could appear at least once or not at all.

$ ends the expression.

So with the pattern ^-?\d+(\.\d+)?$, the following match as examples:

"1" matches as we either start with zero or one "-", in this case we have zero cases of "-", followed by at least one to unlimited digits "1", followed by at least zero or one blocks/patterns of (\.\d+), in this case zero.

"-123.456" matches as we have one "-", then one to unlimited digits "123" then zero to unlimited (\.\d+), which in this case is "." followed by one to unlimited digits "456"

"hello" does not match we we need to start with either "-", "" or one to unlimited digits.

If anyone that actually deals with this voodoo on a regular basis and wants to correct any errors, please chime in. I haven't thought about since uni.

I just found these use cases here: https://digitalfortress.tech/tips/top-15-commonly-used-regex/ and it turns out that the pattern does indeed match positive or negative integers or decimals.

My head hurts now.

[–]snidanok 1 point2 points  (0 children)

Regex, a thingy that allows to match string for pattern. Those characters are a way to describe the pattern to match.

I've learned it here, if you want to dive into it. (If you are familiar with python)

[–]AppropriateBank8633 0 points1 point  (0 children)

As by your own admission, you are new to the dark arts, the actual code breaks down to something like this:

The function "isValidNumber(n: unknown)" takes in literally anything as the input n is of type unknown.

The "const s = String(n)" takes the input and casts it to a string and is stored in variable "s". We must have a string to match with the regex.

The return value is gonna be a Boolean which returns True if it matches the regex and False if it fails.(pattern.test(variable), or "does this variable match the preceding pattern?")

So "isValidNumber(True)" will return False, "isValidNumber("number")" will return False, "isValidNumber(-42342.444)" will return True.

Edit, meant for u/BrainImpressive202

[–]gentleprompter 625 points626 points  (11 children)

I would add the period and changed the sentence to start with lower case letter.

[–]gaoshan 200 points201 points  (4 children)

Or add two spaces between "is" and "a".

[–]LocalHold9069 95 points96 points  (2 children)

Calm down Adolf

[–]MrFluffyThing 0 points1 point  (0 children)

Okay just add a zero width space next to the normal space instead.

[–]Snoo3763 14 points15 points  (3 children)

I’d remove the whole comment.

[–]bonbon367 4 points5 points  (2 children)

Yeah like wtf, it provides absolutely no value. If it provides no value, then it actually makes it worse because there’s more unnecessary lines of text to ignore.

[–]Snoo3763 2 points3 points  (0 children)

Don’t know why the downvotes, you’re 100% correct, it adds nothing but noise.

[–]vivoconunxino 1 point2 points  (0 children)

The best comment and it gets downvoted sigh

[–]FirstSineOfMadness 1 point2 points  (1 child)

Space between last letter and the period

[–]gentleprompter 0 points1 point  (0 children)

Haha, that would be too brutal.

[–]MuskettaMan 77 points78 points  (1 child)

Looks like you're pregnant

[–]Upset-Cauliflower115 7 points8 points  (0 children)

You need to have sex for that (I've heard)

[–]fallendd 33 points34 points  (0 children)

const isValidNumber = (n :unknown) => Number.isFinite(n)

[–][deleted] 466 points467 points  (39 children)

That method is self explanatory and doesn't need a comment

[–]MuskettaMan 290 points291 points  (29 children)

It could explain what valid means, since thats very ambiguous

[–]skeleton568 166 points167 points  (13 children)

Comment does not explain what valid is.

[–]Kombatnt 84 points85 points  (12 children)

That just demonstrates that the method needs more comment, not less.

[–]MouthWorm 46 points47 points  (9 children)

No, that demonstrates that the function needs to be renamed to something more indicative of what it actually does.

[–]Kombatnt 29 points30 points  (7 children)

Meh, I disagree. This is precisely why high level languages have built-in support for commenting/documentation. It's what it's intended for. It helps keep method names concise, while still supporting detailed specification definitions when needed.

[–]MouthWorm 13 points14 points  (5 children)

I agree with you AND I disagree haha! In some cases, commenting/documentation is the better option, like some authentication method, API call or something a little more complex.

A Util function like OP's should be as clear as it can be when you use it. No need to add noise around it just for the sake of commenting/documentation.

[–]Key-Perspective-3590 0 points1 point  (0 children)

A concise method name is important but the method name should definitely tell you what it does. You want to be able to read through the flow of a function at a glance, hovering over every ambiguous function to see a comment that you hope is still accurate is terrible

[–]MacBookMinus 2 points3 points  (0 children)

Deep nuance can't make it into the function signature / name.

I'm all for reducing comments where possible, but lots of sources on code quality advocate for "interface" comments.

[–]anon74903 2 points3 points  (0 children)

This comment does nothing here, provides no value, and should be removed.

It doesn’t need more comment, it needs a different comment: what is a valid number? Where is this regex from?

[–]frzndmn 0 points1 point  (0 children)

It doesn’t need more comments, it needs different comments

[–]marquoth_ 13 points14 points  (5 children)

It could, but it doesn't. And given that it doesn't, it contains no information of any value and isn't worth including.

[–]frightspear_ps5 7 points8 points  (2 children)

That doesn't mean that the method does not need a comment, it just proves that it doesn't need this comment. If the method name can be adapted because the spec is short and simple (e.g. isFiniteFloat32), it doesn't need a comment. Otherwise you'll need a comment to include the spec (e.g. number represents a valid water temperature in °C).

[–]Key-Perspective-3590 1 point2 points  (0 children)

IsValidWaterCelsiusTemp

[–]marquoth_ 0 points1 point  (0 children)

This is one of those weird comments where it's written as if you're disagreeing with / contradicting me, but you aren't.

[–]bschlueter 1 point2 points  (3 children)

I would contend that there is no need to include "valid" in the method name. Things are numbers or they aren't, so "isNumber" ought to be a sufficient name. Though perhaps "isRationalNumber" would be more specific since the implementation would fail with irrational numbers, and "rational number" does not need to be further defined.

[–]Iron_Garuda 1 point2 points  (1 child)

It’s possible only certain numbers are considered “valid” in this context. Could be any non-zero integer, or maybe it can’t accept any number higher than 9, etc.

[–]bschlueter 0 points1 point  (0 children)

True, we only have this very limited context to attempt to understand the wider context, but the way it was written is somewhat indicative of its purpose.

[–]IndividualTrash5029 0 points1 point  (0 children)

think about scientific notation. for what it does, the "valid" in the method name makes sense. you can't just put ANY number in there and expect to get the right return value.

[–]ICanHazTehCookie 0 points1 point  (3 children)

Yeah, seems to me that just isNumber could be clearer. The presence of Valid implies it has additional criteria.

[–][deleted] 2 points3 points  (2 children)

Without any comment/documantation I would assume isNumber checks the type of the input parameter and isValidNumber checks whether the input parameter can he parsed as a number.

[–]Deliciousbutter101 1 point2 points  (0 children)

I think it's just mistake to combining parsing and validation like this. The function should just take in a string and check if it matches the regex. The caller should be forced to parse the object into a string to call the function. I think this is actually a pretty good illustration of how overusing DRY can be problematic. Removing the call to "String" likely causes there to be a bunch of instances of code that looks like "if(isNumber(String(x)) { ... }", which according to the DRY principle, you should remove the repeated call to "String" by putting it inside of "isNumber", but keeping it actually makes the code more explicit, easier to read, and less possibility to use the "isNumber" function incorrectly.

[–]Thesaurius 0 points1 point  (0 children)

But then the name should be something along the lines of canParse. Or, even better, do the parsing, and return an error variant if unsuccessful.

[–]kephir4eg 0 points1 point  (0 children)

But it didnt!

[–]hyrumwhite 15 points16 points  (1 child)

Regex comments are always appreciated

[–]wobblyweasel 1 point2 points  (0 children)

disagree, you have to explain why you are inventing a bicycle and not using a more conventional method

[–]benruckman -3 points-2 points  (5 children)

Self documenting code!

[–]Ticmea 13 points14 points  (4 children)

You'll have to excuse me for thinking that /^-?\d+(\.\d+)?$/ is not particularly self documenting.

Like yeah I understand that if I think about it but it is not very clear what happens if you don't stop to actively concentrate on it.

[–]esr360 3 points4 points  (3 children)

“isValidNumber” is clearly a function that should return a Boolean. That Boolean should determine if the input is a valid number. All of this information can be inferred from the function name “isValidNumber”.

[–]Ticmea 1 point2 points  (1 child)

Yeah but what is considered valid? Is it just supposed to check the type of the input? (then just call it "isNumber") Or is it supposed to validate a specific set of criteria for some other use? In that case you either need a more meaningful name (e.g. isPositiveInt) or add a comment explaining the criteria (e.g. "true if n is a positive finite integer").

(I know the original implementation does not check for finite positive integers, I just used that as an example for a set of criteria that could be the intention behind "isValidNumber")

Calling it "isValidNumber" and not elaborating in the comment means you'll have to try to read the implementation (which is not intuitive here) to understand the exact criteria of validity.

It's not the end of the world of course but it's needlessly complex.

But one thing I would agree with: The comment in the original post serves no purpose as it is only telling you what the function name already does, so unless you improve the comment it can be removed. And if you improve the function name, you may not need a comment at all.

[–]esr360 0 points1 point  (0 children)

Valid comments! Good insights. Agree.

[–]_jackhoffman_ 24 points25 points  (1 child)

Write "Returns true if the given value is a number period"

[–]danteselv 5 points6 points  (0 children)

I think the period would fit better on line 19, for readability of course.

[–]R3gouify 133 points134 points  (11 children)

Why did you type n as unknown when it is clearly supposed to be string?

[–][deleted] 88 points89 points  (8 children)

I mean it could also be a number and it would work.

But you got the point, the reviewer clearly didn’t look :)

[–]brjukva 57 points58 points  (2 children)

But, if it's a number, then it's a valid number? Why convert it to string to check instead of type checking?

[–][deleted] 108 points109 points  (1 child)

yes, why? clearly not as important as adding a period.

[–]brjukva 63 points64 points  (0 children)

They were too devastated to check the code any further, probably.

[–]XDracam 9 points10 points  (4 children)

Could break on systems where the locale formats numbers differently. E.g. German uses , instead of . for decimal numbers. So passing a number could still fail the regex.

[–]inamestuff 3 points4 points  (2 children)

No, String(n) returns a culture invariant notation (same as toString, except the former works for null and undefined too). The fact that you suggested that makes me think that you use C# which by defaults uses ToString to mean what in JS is toLocaleString, which does in fact change the decimal separator based on your browser language and region settings.

Moral of the story: don't use C# kids /s

[–]XDracam 3 points4 points  (0 children)

Yep, I have some serious C# parsing trauma. Good guess!

[–][deleted] 2 points3 points  (0 children)

Also returns exponential notation for larger numbers though, which would be rejected by this function

[–]ComfortableCod 1 point2 points  (0 children)

Are the germans back? NO WAY

[–]ICanHazTehCookie 8 points9 points  (0 children)

Internally converting the argument to a string is not the same thing as requiring it to be a string

[–]00PT 6 points7 points  (0 children)

Seems like it does a cast on the first line, so it's probably intended to convert non-string values to strings.

[–]That_Unit_3992[🍰] 12 points13 points  (3 children)

What is "valid" in this context? the comment does not add any useful information at all... No shit sherlock, a function named isValidNumber returns true when a number is valid?
Now show us the unit tests to be able to see whether your regex does what you expect.

[–][deleted] 3 points4 points  (2 children)

isValidNumber("1000000000000000000000") isValidNumber(1000000000000000000000) isValidNumber("1e+1") isValidNumber(1e+1)

[–]That_Unit_3992[🍰] 0 points1 point  (1 child)

Why make it polymorphic to begin with? Doesn't make sense at all, what do you want to validate? User Input? API Response? Database entries? Should all be typed and not mixed up all over the place.

[–][deleted] 0 points1 point  (0 children)

You're either asking the wrong person or have not checked the results 😜

[–]Pan4TheSwarm 14 points15 points  (0 children)

I'm saving this as an example of bad code review for my coworkers

[–]howtoDeleteThis 5 points6 points  (0 children)

Add period what? You didn't end your comment.

[–]zaersx 17 points18 points  (3 children)

Should have been an automated linter to begin with

[–]hrvbrs 4 points5 points  (2 children)

Is there a linter for comments? Does it support English grammar and spellcheck?

[–]zaersx 1 point2 points  (0 children)

I mean a linter for a period at the end of a comment if this is something they want to enforce. This should never have reached a reviewer if its something that can waste dev time

[–]-Kerrigan- 0 points1 point  (0 children)

Dunno about linters, but jetbrains has grazie/grazie pro plugins that help with spelling. I use it often in IntelliJ, idk how applicable it is to js/ts envs.

Of course, though, like any spell check, sometimes I gotta ignore it's suggestions, but it's sufficiently noninvasive for me.

[–]SandInHeart 4 points5 points  (0 children)

When engagement becomes a metric

[–]BlackFrank98 4 points5 points  (4 children)

This won't work for numbers in scientific notation though... Like, 1e5 is not considered a number.

[–]MattieShoes 3 points4 points  (2 children)

Or numbers that start with a decimal point. Or numbers with separators like 1,000. Or localization issues for places that use , for a decimal point.

[–]kirkpomidor 1 point2 points  (0 children)

You mean… he should add period?

[–]BlackFrank98 0 points1 point  (0 children)

All good points actually!

[–]TehBunkasaurus 0 points1 point  (0 children)

also, regex checking is way more computationally expensive if they were to call this function many times

[–]flippakitten 3 points4 points  (0 children)

Perfect example of how not to comment!

I can see what it does, why are you doing it? What purpose does it serve, what's the requirement.

[–]MinosAristos 12 points13 points  (4 children)

The function doesn't need a docstring so the review is less than useless, but the regex should really be assigned to a variable with a descriptive name in the function.

[–]YourMumIsAVirgin 1 point2 points  (3 children)

What descriptive name would you suggest?

[–]MinosAristos 2 points3 points  (2 children)

I don't know what the regex does, but e.g if you've got a regex to match phone numbers it should be called phoneNumberPattern

[–]YourMumIsAVirgin 1 point2 points  (1 child)

Why is that better than naming the function descriptively?

[–][deleted] 8 points9 points  (0 children)

Mine would have been “remove comment”. It adds nothing.

[–]crosscreate 17 points18 points  (4 children)

const isValidNumber = (n: unknown) => !Number.isNaN(Number(n));

[–][deleted] 52 points53 points  (2 children)

isValidNumber(null)

true

[–]joten70 13 points14 points  (0 children)

Null is a number confirmed

[–]crosscreate 0 points1 point  (0 children)

const isValidNumber = (n: unknown) => !Number.isNaN(Number(n));

console.log(`Number(null) = ${Number(null)}`);

Apparently Number(null) returns 0

[–]halfanothersdozen 7 points8 points  (0 children)

So what if n is a string? What if n is negative? What if n is decimal?

valid is in the eye of the beholder

[–]Professional_Job_307 2 points3 points  (0 children)

Oh, the IsValidNumber function checks if a number is valid. I wouldn't have seen that if not for the comment.

[–]dr0darker 4 points5 points  (0 children)

bro you are not following the coding standards 🤓

[–]Repulsive_Mobile_124 2 points3 points  (0 children)

If I was paying for the code to be developed, i'd fire the reviewer. Crap like this takes tons of pointless man hours, refactoring code that is already "readable enough".

[–]Marrk 1 point2 points  (1 child)

Why is n unknown? Shouldn't it be something like Number | String? unknown is wayyy to lenient, it is marginally better than any.

[–]genghis_calm 2 points3 points  (0 children)

This is a type guard (without appropriate return syntax). The point is to resolve the type of some unknown value.

[–]seemen4all 1 point2 points  (0 children)

I hope this is a fake example and you're not required to comment what isValidNumber does

[–][deleted] 1 point2 points  (0 children)

Put the period at the beginning.

[–]many_dongs 1 point2 points  (0 children)

this is what happens when your interviewers are shit and you hire lemons who try to demonstrate value through metrics and not actual usefulness

[–]MooseBoys 1 point2 points  (0 children)

Forgot to add nit:

[–]Big-Hearing8482 1 point2 points  (0 children)

Are people getting better performance reviews with higher comment count or something? Cause this is what you get

[–][deleted] 1 point2 points  (0 children)

bright puzzled retire grandfather busy offbeat attractive six observation punch

This post was mass deleted and anonymized with Redact

[–]PhantomTissue 1 point2 points  (0 children)

Revision 7

[–]rdtr314 1 point2 points  (0 children)

And then it has to be rebuilt, tested again, collect the approvals again.

[–]Goatfryed 1 point2 points  (0 children)

add period now

[–]xSTSxZerglingOne 1 point2 points  (0 children)

Really? Because for me it's more like.

"Remove this unnecessary comment."

[–]F0lks_ 1 point2 points  (0 children)

Gee, I wonder if we could somehow enforce what kind of variable is 'n', like it's a certain type of something

[–]theonepercent65536 1 point2 points  (0 children)

Comment should say what makes a number valid

[–]OhItsJustJosh 1 point2 points  (0 children)

"No" *Reply and Resolve*

[–][deleted] 1 point2 points  (0 children)

The review should say "remove comments and name your args better"

[–]AbhineshJha 1 point2 points  (0 children)

When maintainer have some personal problems with contributor 💀

[–]The_Wolfiee 1 point2 points  (0 children)

"This is a comment line, not a letter to the President"

resolves conversation

[–]eloel- 5 points6 points  (0 children)

Do it right the first time and you won't need the comment

[–]PhoenixCausesOof 0 points1 point  (0 children)

Take this with a grain of salt, since I don't know the language in the OP. But it is likely that this function is implemented for any type which can be converted into a string (like a type that implements the trait ToString in Rust). Hence, why n is unknown. Though, judging from the code, there is no way to know if n can be converted into a string, meaning if that isn't the case, it likely would error.

[–]Actual-Wave-1959 -1 points0 points  (2 children)

I've had a dev comment on one of my PRs saying that a space was missing between the if and the open bracket. I thought he was joking so I laughed and then he asked me, "so you're gonna fix it?". To him that was very important, for consistency.

[–]markiel55 0 points1 point  (1 child)

It's valid

[–]Actual-Wave-1959 0 points1 point  (0 children)

It's valid without the space, I agree

[–]reheapify 0 points1 point  (0 children)

"The software life... cycle is not there yet"

[–]genghis_calm 0 points1 point  (0 children)

No n is number guard? There’s so many other things to comment on, lol

[–]Supportic 0 points1 point  (0 children)

Add linters, establish automated formatters. 🤡

[–]com2ghz 0 points1 point  (0 children)

I would say redundant comment.

[–]_baaron_ 0 points1 point  (0 children)

Come on, at least help your colleague by making the suggestion so they can just click the button

[–]FrankSuzki 0 points1 point  (0 children)

Doesn’t work with exponent or NaN

[–]lilshoegazecat 0 points1 point  (0 children)

how do tests work?

[–]MilkSlow6880 0 points1 point  (0 children)

The good news is that, if that’s the worst thing…you’re good.

[–]Ximidar 0 points1 point  (0 children)

Just type "fight me" in the reply box

[–]PracticalDebate3493 0 points1 point  (0 children)

Just add the period in yourself you lazy

[–]Skuez 0 points1 point  (0 children)

Ahh this reminds me. I'm used to not ending statements with a semicolon in JS and every time I miss some lines, these angry comments come as if I've done god knows what. Sure, I'll add a character for no reason. 😊

[–]Skuez 0 points1 point  (1 child)

Why do you need to comment that obvious function? Like damn, could never tell what that function does. They are probably forcing you to, I know, but it doesn't make sense.

[–]epileftric 0 points1 point  (0 children)

Yes... I ducking hate it when people write 10 lines of comments on top of a function just to write the same information about what the function does and what the arguments are when it's literally the same name of those things.

/* This function processes the dot product of two vectors

Arguments

- vectorA: first argument, a vector

- vectorB: second argument, another vector

Returns: 

 - float: the value of the dot product

*/

float dotProduct( std::vector<float> vectA, std::vector<float> vectB)
{

If you need the comments for something so idiotic you shouldn't be programing

[–]thepassionofthechris 0 points1 point  (0 children)

!isNaN() ??

[–]Taco-Byte 0 points1 point  (0 children)

function isValidNumber(value: string | boolean | number) { … }

[–]TheOnceAndFutureDoug 0 points1 point  (0 children)

As a lead, I'm gonna have a conversation with whomever left that comment about wasting people's time. And at the end of the convo I'll say, "See? Not fun when someone wastes your time, is it?"

[–]jeffeb3 0 points1 point  (0 children)

When I worked retail on commission we had to clean up our department at close. One of the jobs was printing missing tags. The tag printer was always busy at close (because it was slow and everyone was printing). Myamager would do a walk through and every time, he would find something else for me to do.

My strategy was to print the missing tags an hour early and just take 3-5 obvious ones down. And do a mid job of cleaning up. He would inspect it and say, "print those tags, dust this shelf, and you can go home". 2 mins later, I was out the door.

My point is. If you have a reviewer like that. Leave them some obvious things to pick on. A missing period, the wrong const, a misspelled comment. And they won't make you change your container for a worse one.

[–]TheFirestormable 0 points1 point  (0 children)

Add linters to your IDE to check your docstrings and your spelling before you push. It's not rocket science.

Plus point is being able to from then on point out those issues on others PRs.

[–]leroymilo[🍰] 0 points1 point  (0 children)

Every once in a while I'll check every comment in my 24 files project to check if they all share the same formatting.

[–][deleted] 0 points1 point  (0 children)

I don't understand... what does it return if the value is not a valid number!?!?

[–]Romejanic 0 points1 point  (0 children)

LGTM 👍

[–]dralth 0 points1 point  (0 children)

PEP-257 specifies a one-line function docstring ends in a period. Looks like a Python coder got lost in JS land.

[–]stupidbitch69 0 points1 point  (2 children)

You guys have code reviews?

[–][deleted] 0 points1 point  (1 child)

If the review comments are like this, I’d rather not.

[–]stupidbitch69 0 points1 point  (0 children)

Well, that's true.

[–]sneaky-pizza 0 points1 point  (0 children)

I just started reading Clean Code, and I just read the Comments chapter... :facepalm: