this post was submitted on 05 Sep 2024

53 points (93.4% liked)

Programming

17313 readers

102 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Follow the programming.dev instance rules
Keep content related to programming in some way
If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities [email protected]

founded 1 year ago

MODERATORS

[email protected]

Code Smells Catalog (luzkan.github.io)

submitted 2 months ago by [email protected] to c/[email protected]

21 comments fedilink hide all child comments

all 22 comments

sorted by: hot top controversial new old

[–] [email protected] 10 points 2 months ago* (last edited 2 months ago) (3 children)

The items don't seem concise and always clear. But seems like a good, inspiring resource for things to consider.

If it is expected that a method might fail, then it should fail, either by throwing an Exception or, if not - it should return a special case None/Null type object of the desired class (following the Null Object Pattern), not null itself.

I've never heard of evading null with a Null object. Seems like a bad idea to me. Maybe it could work in some language, but generally I would say prefer result typing. Introducing a result type wrapping or extending the result value type is complexity I would be very evasive to introduce if the language doesn't already support result wrapper/state types.

[–] [email protected] 2 points 2 months ago (1 children)

I’ve never heard of evading null with a Null object.

This is quite standard, and in fact it's even a safety feature. C++ introduced nullptr defined as an instance of std::nullptr_t explicitly with this in mind.

https://en.cppreference.com/w/cpp/language/nullptr

This approach is also quite basic in monadic types.

[–] [email protected] 3 points 2 months ago* (last edited 2 months ago) (2 children)

with this in mind

With what in mind? Evading NULL?

Languages that make use of references rather than pointers don't have this Dualism. C# has nullable references and nullability analysis, and null as a keyword.

What does your reasoning mean in that context?

[–] [email protected] 5 points 2 months ago* (last edited 2 months ago)

Languages that make use of references rather than pointers don’t have this Dualism.

It's not about references vs pointers. You could easily have a language that allowed "null references" (edit: too much C++; of course many languages allow null references, e.g. Javascript) or one that properly separated null pointers out in the type system.

I agree with your point though, using a special Null value is usually worse than using Option or similar. And nullptr_t doesn't help with this at all.

[–] [email protected] 0 points 2 months ago (1 children)

With what in mind? Evading NULL?

Depends on your perspective. It's convenient to lean on type checking to avoid a whole class of bugs. You can see this either as avoiding NULL or use your type system to flag misuses.

Languages that make use of references rather than pointers don’t have this Dualism. C# has nullable references and nullability analysis, and null as a keyword.

C#'s null keyword matches the monadic approach I mentioned earlier. Nullable types work as a Maybe monad. It's the same concept shoehorned differently due to the different paths taken by these languages.

[–] [email protected] 1 points 2 months ago* (last edited 2 months ago) (1 children)

as far as I know, C# don't have proper ergonomic monadic bind as in F# (computation expression), Haskell (do expression), and Ocaml (let*), but I could be wrong.

[–] [email protected] 1 points 2 months ago

Correct.

[–] [email protected] 2 points 2 months ago

This might be educational: https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html

There are issues that the Optional class alleviates that are common enough to be documented: https://www.jetbrains.com/help/inspectopedia/ConditionalCanBeOptional.html (more detail is available at places like https://github.com/JetBrains/intellij-community/blob/a2d32ec64ed0fb37c7cc97856aa94cce95b17ee5/java/java-impl/src/inspectionDescriptions/ConditionalCanBeOptional.html (I believe this information used to be visible with the "inspectopedia" URLs but I don't see that today))

On the other hand, it seems there are some features / situations that require null to be present: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining https://www.jetbrains.com/help/inspectopedia/OptionalToIf.html

[–] [email protected] 7 points 2 months ago (1 children)

This is like SCPs but for devs

[–] [email protected] 1 points 2 months ago

You're thinking of CVEs.

[–] [email protected] 6 points 2 months ago (3 children)

This doesn't seem overly useful.

It's a list taken out of a bunch of books with no regard for how something can be the best path in one language and a smell in another language.

Look at this page for example: https://luzkan.github.io/smells/imperative-loops

It suggests using functional loop methods (.map(), .reduce(), .filter()) instead of using imperative loops (for, for in, for each) but completely disregards the facts that imperative loops also have access to the break, continue, and return keywords to improve performance.

For example: If I have an unsorted list of 1000 cars which includes a whole bunch of information per car (e.g. color, year manufactured, etc...), and I want to know if there were any cars were manufactured before the year 1980, I can run an imperative loop through the list and early return true if I find one, and only returning false if I haven't found one by the end of the list.

If the third car was made in 1977, then I have only iterated through 3 cars to find my answer.

But if I were to try this with only functional loops, I would have to iterate through all 1000 cars before I had my answer.

A website with blind rules like this is going to lead to worse code.

[–] CookieOfFortune 11 points 2 months ago

That’s a pretty bad example since most functional frameworks include an any or some function that returns early.

[–] [email protected] 8 points 2 months ago (2 children)

..what? At least with Java Streams or Kotlin Sequences, they absolutely abort early with something like .filter().first().

[–] [email protected] 10 points 2 months ago (1 children)

Same in Python, Rust, Haskell and probably many others.

But apparently JS does work that way, that is its filter always iterates over everything and returns a new array and not some iterator object.

[–] [email protected] 3 points 2 months ago* (last edited 2 months ago)

The old methods on Array will eagerly evaluate all elements. But JS has a new Iterator type with methods that works lazily instead.

[–] [email protected] 5 points 2 months ago (1 children)

Ya, streams may seem tedious (why do I have to call stream and collect?), but it’s like that for performance (and probably backwards compatibility).

If writing readable code is not peformant, then the language implementation needs to be fixed.

[–] [email protected] 5 points 2 months ago* (last edited 2 months ago) (1 children)

Honestly, it is much more code to use loop with non-local control like break, continue etc. (variable initialization, append, variable mutation in loops...) than just calling a collect function (which I assume just means to_list). In the above example, in most programming language I know, you don't even need to collect the result into a list.

Not to mention, large loops with non-local control is a breeding ground for spegatti code. Because you no longer have a consistent exit point to the loop, thus making the semantics hard o reason about.

In many languages, there are type class / trait / interfaces (whatever you want to call them) that allows lazy structures to share the same API as strict ones.

[–] [email protected] 3 points 2 months ago (1 children)

Yeah, in Java calling first() on a stream is the same as an early return in a for-loop, where for each element all of the previous stream operations are applied first.

So the stream operation

cars.stream()
    .filter(c -> c.year() < 1977)
    .first()

is equivalent to doing the following imperatively

for (var car : cars) {
    if (car.year() < 1977) return car;
}

Not to mention Kotlin actually supports non-local returns in lambdas under specific circumstances, which allows for even more circumstances to be expressed with functional chaining.

[–] [email protected] 4 points 2 months ago (1 children)

These are not quite equivalent. In terms of short-circuiting yeah they both short-circuit when they get the value. But the latter is returning from the current function and the former is not. If you add a return to that first example then they are equivalent. But then cannot be used in line. Which is a nice advantage to the former - it can be used inline with less faff as you can just assign the return to a value. The latter needs you to declare a variable, assign it and break from the loop in the if.

Personally I quite like how the former requires less modification to work in different contexts and find it nicer to read. Though not all logic is easier to read with a stream, sometimes a good old for loop makes the code more readable. Use which ever helps you best at each point. Never blindly apply some pattern to every situation.

[–] [email protected] 4 points 2 months ago

Well yes, I was simplifying because I wanted to address the main (incorrect) criticism by @[email protected]. I agree with your comment

[–] [email protected] 2 points 2 months ago

Also, Effective Java specifically says to use streams judiciously and prefer traditional for loops in general.