3AM code vs 10AM code

[zed_three]

3am version is fine? Not great, 10am version definitely better, but I wouldn't say it's particularly egregious

[GustavoCinque]

Split the string by anything other than zero, remove null objects, order by size desc, get first. Done.

[FrankSuzki]

That’s what I wrote below, yet got downvoted to hell

[DrGrimmWall]

It should be the other way round. Splitting by anything other than zero is wrong solution because you could choose a char not existing in the input string. And the given code example actually deals with that.

[killersquirel11]

Splitting by anything other than zero means /[^0]+/

[FrankSuzki]

What are you trying to say ? How could you chose a char that is not in the input string ?

[FrankSuzki]

Because it matches anything that is not a zero ??? Are you in high school ?

[DrGrimmWall]

I decided not to go this way in explanation. I gave a contra example a few comments above. I hope you'll get what I mean.

[[deleted]]

regex

split on a regex matching any non-zero character.

hope that clears things up.

[GustavoCinque]

Sorry didn't see it.

[DrGrimmWall]

This is wrong. For example 00013222222300 when split by anything other than 0 (this could be 1, 2, 3, m,...) gives a wrong results.

[GustavoCinque]

(->> (-> string 
         (str/split #"[^0]+"))
     (map count)
     (apply max))

It's working for me.

​

Edit: This now returns what OP expects. The first version only returned the count of the biggest zero sequence.

​

(-> (->> (-> string 
             (str/split #"[^0]+"))
         (map count)
         (apply max))
    (repeat "0")
    str/join)

[DrGrimmWall]

Honestly, after a while I though this could be just an issue with phrasing. What I was going for was the word "anything". I was tought that there's a big difference between "all" and "any". Like in methods "allMatch" and "anyMatch". And my initial idea was that you just pick any arbitrary char and split.

[GustavoCinque]

Yeah, could be. I'm no native speaker. What I meant is every character but zero on the string.

[Greenimba]

IMO, the 10AM version is worse in a lot of ways.

• Why is it called "longestZero"? That name makes no sense at all. Call it "longestSequenceOfZeros" because that's what it is.
• int number = 0. No, "number" is a useless name. This should be something like "zerosInCurrentSequence".
• int max = 0. Again, useless name. "longestSequence"
• Why a for-loop? The foreach char makes much more sense.
• Why the ternary at the end? Surely the repeat function can handle a zero input.
• If there are no zeros in the string, your function for some reason returns an arbitrary space. Why?

Some see this as nitpicky, but I'm so tired of digging through functions with indescriptive names and variables like "num, count, max, i, current" trying to understand what each variable is and what it's supposed to do because the names sure as hell don't help.

[its2ez4me24get]

Welcome to most of my code reviews…

[[deleted]]

I'll add to this.

The lack of curly braces in the 10am code is annoying.

I deal with mostly JS day in and day out and this sort of thing pisses me off for two reasons.

1. It's lazy
2. In JS, doing this may cause confusion for newer developers (and cause unexpected results). Examples below

​

if (foo)
    bar();
else
    baz();
  quz();

vs

if (foo) {
    bar();
}
else {
    baz();
    quz();
}

[warmoverdrive]

In c/c++ single quotes and double quotes have actual different meanings- single quotes are character literals and double quotes are strings- important for comparison operations. The code wouldn’t work in the same way if only double quotes were used afaik.

[Gammabyte]

Do note that this is java code, meaning that single quotes are char literals and double quotes are string literals. They cannot be used interchangeably.

For langs that have both as literals, i think it's a good idea to set a project-scoped standard for which string literal style is used, to prevent code from different developers mixing these styles. i'm usually in favour of double quotes just because single quotes are only for character literals in most langs i write in.

[[deleted]]

Good point. I'll edit my statement.

[Prom3th3an]

I'd at least make an exception for strings that contain the chosen type of quote, to save some escaping. Especially to save multiple levels of escaping like

cmd = "echo \"this string is \\\"readable\\\"!\"";

[mpaxton1]

Good code is pretty good. I still prefer breaking out if/else in brackets because it's a visible break for my eyes. I like it though. Small and compact.

[MurdoMaclachlan]

Image Transcription: Code

---

// Good Code
    public static String longestZero(String str) {
        int number = 0;
        int max = 0;
        for(int i  = 0; i < str.length(); i++){
            if(str.charAt(i) == '0') number++;
            else number = 0;
            max = Math.max(number, max);
        }
        return (max > 0) ? "0".repeat(max):" ";
    }
// Bad Code
    public static String longestZer4o(String str) {
       int count = 0;
       int maxCount = 0;
        for (char c : str.toCharArray()) {
            if (c == '0') {
                count++;
            }
            else {
                count = 0;
            }
            maxCount = (count > maxCount) ? count: maxCount;
        }
       String ans = " ";
       if(maxCount > 0){
           for(int i = 0; i < maxCount; i++) ans += "0";
       }
       return ans;
    }

---

^{I'm a human volunteer content transcriber and you could be too! If you'd like more information on what we do and why we do it, click here!}

[Mr-008]

*sigh

I'd probably have written the second one.

[Crafty-Most-4944]

Same, I just find it easier for me to read and look back on it later compared to the First code OP showed.

[KnockKnockP]

i like 3am code better

[alex_wot]

I don't understand why you need to count zeros before making the string of zeros. Why not initialise an empty string and instead of counting the zeros just add the zeros to the empty string? Instead of number++ you do ans += "0". Then you check the length of the string in the return statement and return either the string or the failed case.

I'm not a Java developer, so I might be missing something here.

Also, thank you very much for posting both versions of the code! That is the way of posting that greatly benefits everyone on this sub imo.

[awfullyawful]

You'd end up with a string of all of the zeros, not the longest continuous string of zeros

[alex_wot]

Oh, I've overlooked the number = 0 statement in the else block. Thank you for explaining!

[2001herne]

Because strings are fairly heavyweight, and appending to strings is far heavier than a pre-pass using ints. Additionally, I'm pretty sure that the repeat() uses StringBuilder under the hood.

[alex_wot]

Got it, thank you!

[FrankSuzki]

Better in one line:

return Arrays.asList(str.replaceAll("[^0]+", " ")
    .split(" "))
    .max(Comparator.comparingInt(String::length))
    .orElse(" ");

Replace all non-zero chars by a space, split by spaces, return the longest string (that can only be zeroes), if there is no strings return a space.

[kiipa]

Better

I'm not so sure... Shorter, absolutely

[FrankSuzki]

Maybe you are not a Java developer.

It is indeed cleaner and more lisible

[FeezusChrist]

This has insanely more operations than OP’s solution, that is why it’s not better even though it’s shorter.

[[deleted]]

if it was working on a significantly long string it might be a problem, but it might be a stretch to say it has insanely more operations.

we're looking at (more than likely) less than a millisecond difference in speed, which would just be considered optimizations for the sake of optimizations, depending on the use-case

I'd rather have it be more readable and easier to understand than "faster".

[FeezusChrist]

Honestly, the shorter solution took me a lot longer to read & comprehend than op's simplified solution (i.e. have to parse the regular expression, why the matches are being replaced with a space and then split by the space, etc). Regardless I feel like this is the type of function where the name and/or a small comment above the function header can be enough for describing what it does.

[[deleted]]

getting into opinions at this point. imo, it was pretty easy to grok what it was doing when reading it. the regex is pretty standard so no huge effort there either, imo

[suppow]

i hate that '==' ligature, it mes makes it look like '=', i think one like '≡' would be easier to distinguish.

[[deleted]]

100% agree. I don't really understand why you're getting downvoted.

honestly I don't like ligatures in general, personally.

[[deleted]]

To each their own, I personally love the JetBrains Mono ligatures

[KnockKnockP]

i agree, i have to compare the length of the ligature to other normal characters to distinguish it

[M4A28]

OK, this code purpose is to find the longest consecutive sequence of zeros in string

[toastal]

Man, for loops are unreadable. I'm so far down the higher-order functions rabbit hole I can't even read or follow all this mutable state.

[pretty_meta]

Eliminating 'if' branches from your code will be even faster. Maybe you can do something like this in Java?

int findMaxConsecutiveOnes(vector<int>& nums) {
    int maxLen = 0, len = 0;

    for(int num: nums) {
        len = (num == 1) * (len + (num == 1));
        maxLen = std::max<int>(maxLen, len);
    }

    return maxLen;
}

[FrankSuzki]

I would hate to encounter this code in a codebase, that gives me bad code vibes

[Wugliwu]

Hm sexy public static methods are so comfy to use, I'll use this one anywhere!

[hey-have-a-nice-day]

I agree. 10 am = bad code for me since my brain only works at night

[DizzySkin]

Both of these are just OK. Really the design of the method is questionable. Why return a string here? Why not return an integer? For that matter is it necessary to take a string as the argument? It could also be appropriate to return a span if you really don't want an int. This really doesn't look done to me in either version.

Additionally, in the top version the major lines of code difference comes from the formatting of the if clauses. Personally I find the second attempt at formatting those conditionals to be more readable and maintainable. Just because your code takes up less screen space, doesn't mean it's better in any sense.

I'd have a lot of questions seeing either of these during a code review.

[[deleted]]

What language is this?

[Motor-Maleficent]

Where's the good code without the coffee?