DEV Community: Shani Fedida

How to get rid of NullPointerException

Shani Fedida — Sat, 12 Sep 2020 17:39:53 +0000

NullPointerException — it surprises me the same as it surprises you

OverOps, an Israeli company that helps developers understand what happens in production, carried out research on what are the top Java exception in production. Want to guess which one is in #1 place? NullPointerException.

NullPointerException OverOps’s monster

Why is this exception is so frequent? I argue (as does Uncle Bob in his book Clean Code 😉) that it is not because developers forget to add null checks.

The reason: developers use nulls too often.

So where do all those NULLs come from?

In C# and Java, all reference types can point to null. We can get a reference to point to null in the following ways:

“uninitialized” reference type variables — variables that are initialized with nulls and are assigned with their real value afterward. A bug can cause them to never be reassigned.
uninitialized reference-type class's members.
explicit assignment to null or returning null from a function.

Here are some patterns I noticed in functions returning null:

Error handling

Returning null when the input is invalid. This is one way of returning error codes. I think it is an old school programming style, originating in the times when exceptions didn’t exist.

Missing optional data for entities

An entity’s property can be optional. When there is no data for an optional property, it returns null.

Hierarchical models

In hierarchical models, we usually can navigate up and down. When we are at the top, we need a way to say so, usually we do it by returning null.

Find functions

When we want to find an entity by criteria in a collection, we return null as a way to say the entity was not found.

What are the problems with using nulls?

It will blow up. Eventually…

The code in which the NullPointerException is raised can be very far from where the bug is. It makes tracing the real problem harder. Especially if the code is branched.

I am happy now but I will blow up eventually.

In the following code example, there is a bug, somewhere in class A, causing entity to be null. But the NullPointerException is raised inside a function of class B. Real-life code can be much more complicated.

Hidden errors

I encounter null checks which seem like the developer was thinking:

“I know I should check for null but I don’t know what it means when the function returns null and I don’t know what to do with it,” or
“ I think this cannot be null but just to make sure, I don’t want it to blow up in production”

It usually looks like this:

Those kinds of null checks cause some code logic to not trigger, without the ability to know about it. Writing this kind of code means that some logic of a flow failed but the whole flow succeeded. It also can cause a bug in some other functionality which assumed the other function did its job.

Imagine you buy a ticket to a show online. You got a success message! The day of the show finally arrived, you leave work early, arrange a babysitter, and go to see the show. When you arrive you discover you don’t have tickets! and there are no empty seats. You return home upset and confused😠. Can you see how this kind of null check can cause this situation ?

It also makes the code branched and ugly 😢

Missing non-nullable reference types in C# and Java

In C# and Java reference types can always point to null. This leads to a situation that we cannot know, by looking at a function signature, if null is a valid input or output of it. I believe most of the functions don’t return or accept null.

Because it is hard to know if a function returns null or not (unless documented), developers are either inserting null checks when not needed or don’t check for nulls when needed — and yes, sometimes putting null checks when needed 😉.

This poor design choice causes the problems I described before in “Hidden errors” and a lot of NullPointerException errors, of course. Lose-lose situation. 😢

There are languages like Kotlin that aim to eliminate NullPointerException errors by differentiating between nullable references and non-nullable references. This allows catching the null assigned to non-null references and making sure developers check for null before dereferencing nullable references, all at compile-time.

Microsoft is adopting the same approach by introducing Nullable Reference Types in C#8.

So what should we do?

Listen to Uncle Bob

Robert C. Martin, who is widely known as “Uncle Bob,” wrote one of the most famous books about clean code called (surprisingly) “Clean Code”. In this book, Uncle Bob claims, we should not return nulls and should not pass null to a function.

But how?

I want to propose some technical patterns for eliminating null usage. I am not saying this is the best solution for every scenario — just options.

Using the option type

The option type is a different way to represent an optional value. This type asks if a value exists and, if so, accesses the value. When trying to access the value which doesn’t exist, it raises an exception. This solves the problem of NullPointerException raised in code areas away from the bug. In Java there is the Optional class. In C# (until C# 7 ) there is the Nullable type which is only for value types but you can create your own or use a library.

A straightforward approach is to replace a reference that can be null (by logic) with this type:

Splitting the function into two

Each function that returns null will be converted into two functions. One function with the same signature that throws an exception instead of returning null. The second function returns a boolean representing if it is valid or not to call the first function. Let’s see an example:

If the code holding an IEmployee instance assumes this employee has a manager the code should call to Manager. But if this assumption doesn’t exist the code should call to HasManager and handle the two possible outputs.

Let’s see another example:

The logic of ContainsEmployeById is the same as FindEmployeById but without returning the employee. Now let’s say that those functions reach the DB, we have a performance problem here. Let’s introduce a similar but different pattern: the boolean function when returning true will also return the data we search for. It looks like this:

A common use of this pattern is int.Parse and int.TryParse.

The fact that I can separate a function to two functions and each has its usages is a sign that returning null is a code smell for violating the Single Responsibility Principle.

Splitting the interface

A practical guideline we can derive from the Liskov principle is that a class must implement all the functions of an interface it implements. Returning null or throwing an exception are ways to not implement a function. So returning null is a code smell for violating the Liskov principle.

If a class can’t implement a specific interface’s function we can move that function to another interface and each class will implement only the interface it can.

Now instead of asking employee.HasManager — which we will do if we used the first approach “Splitting the function into two” — we ask employee is IManagedEmployee.

I am not working alone and not on a greenfield project. What now?

In existing codebases, there is a lot of code returning reference types. We cannot know if null is a valid output or not.

The first quick win I wish you to have is to change your coding conventions so null is not a valid input or output to a function. Or, at least when you decide that null is a valid output, use the Option type.

Some tools can help to enforce this convention like ReSharper and NullGuard. I guess, although I haven’t tried this yet, you can add a custom rule to SonarQube which will alert when the word null appears.

I would love to know what you think. Are you going to embrace this convention? And if not, why? What’s holding you back?

If you encounter a scenario in which you think returning null is the right design choice, or the patterns I suggested are not good, I would love to know.

Thanks Mark Kazakov‏ for the funny meme, Alex Zhitnitsky from OverOps for answering my questions, Baot for organizing a great writing event for new bloggers, Itzik Saban , Amitay Horwitz and Max Ophius for giving me feedback.

The 4th step in TDD

Shani Fedida — Wed, 09 Sep 2020 18:47:05 +0000

If you are doing TDD or not, your daily work is combined from a few repetitive actions. Each one has its own “methodology”, even if he is not aware of.

Yesterday I have been working on a bug from production. I created a test reproducing the bug, meaning I had a failing test — the 1st step in TDD.

While working, I found myself thinking about a lot of things in the same time: refactoring, trying to understand the existing code, how I make my test pass without breaking the others, how my tests should look like and so on.

I got into a SWIRL.

Photo by Cassidy Kelley on Unsplash

When I noticed that, I stopped and said to myself ”Shani, one of the reasons you like TDD is because it makes you focus better. Currently, you are not focused at all”.

So I decided to focus on just making my test pass, ignoring that others will fail. And on that occasion make sure they do fail.

If you think about this, I split TDD 2nd step into two:

Make my test pass
Make all the previous tests pass

Why I did this? Because I noticed thinking about those two together was mentally too much for me. (The number of objects an average human can hold in working memory is 7 ± 2, I think for me is just 1 😅)

So here it is, after a search for the relevant entry point for my code, I added one line with hard-coded values relevant only for my test. Happily, Now some tests are red, time for the hard part: making it all work again.

start from a failing test and then make only my test pass

If my changes didn’t cause any tests to fail, it means I didn’t have tests testing different scenarios that were affected by my change (besides the bug scenario). Since I added one line with hard-coded values it probably should break some tests. If that was the case I should have added some green tests before I changed the code in order to catch regressions.

Would love to hear your thoughts and about your personal methodologies.

Is the C# internal keyword a code smell?

Shani Fedida — Fri, 04 Sep 2020 07:36:41 +0000

Background photo created by freepik - www.freepik.com

In this post, I am going to show why I think the internal keyword, when put on class members, is a code smell and suggest better alternatives.

What is the internal keyword?

In C# the internal keyword can be used on a class or its members. It is one of the C# access modifiers. Internal types or members are accessible only within files in the same assembly.

Why we need the internal keyword?

“A common use of internal access is in component-based development because it enables a group of components to cooperate in a private manner without being exposed to the rest of the application code. For example, a framework for building graphical user interfaces could provide Control and Form classes that cooperate by using members with internal access. Since these members are internal, they are not exposed to code that is using the framework.” (C# internal keyword documentation)

These are the use cases I saw for using the internal keyword on a class member:

Call a class’s private function within the same assembly.
In order to test a private function, you can mark it as internal and exposed the dll to the test DLL via InternalsVisibleTo.

Both cases can be viewed as a code smell, saying that this private function should be public.

Let's see some examples

Here is a simple example. a function in one class wants to access a private function of another class.

The solution is simple — just mark A::func2 as public.

Let's look at a bit more complex example:

What’s the problem? just mark func2 as public as we did before.

But we can’t 😞. B is an internal class so it cannot be part of the signature of a public function of a public class.

Those are the solutions I found, ordered by easiness:

Mark the function with the internal keyword
Create an internal interface
Extract A.func2 to another internal class and use it instead of A.func2.
Decouple the function from internal classes and make it public. This is much dependant on what the function is doing with its inputs. decouple the internal classes can be very easy, very hard, and even impossible (without ruing the design).

But we don’t have public classes we use interfaces…

Let's look at some more real-world example:

Let's see how the previous solutions are adapted to this example:

Mark function with Internal. this means you will need to cast to class in order to call the function so This will work only if class A is the only one that implements the interface, meaning IA is not mocked in tests and there isn’t another production class that implements IA.
Create an internal interface that extends the public interface.
Extract A.func2 to another internal class and use it instead of A.func2.
Decouple the function from internal classes and add it to the public interface.

We can see that the internal keyword is the easiest solution, but there are other solutions using the traditional building blocks of OOP: classes and interfaces. We can see that the 2nd solution — adding an internal interface is not much harder than marking the function with the internal keyword.

Why not use the internal keyword?

As I showed in the previous examples, using the internal keyword is the easiest solution. But you are going to have a hard time in the future if you will need to:

Move the public class A to another DLL (since the internal keyword will no longer apply to the same dll)
Create another production class that implements IA
Mock IA in tests

You may think “But this is just one line of code, I or anyone else can change it easily if needed”. Now you have one line of code that looks like that:

((MyClass)a).internalFunction

but if others will need to call this function too this line will be copy-pasted around inside the DLL.

My Conclusion

I think marking a class member with the internal keyword is a code smell. In the examples I showed above it is the easiest solution, BUT can cause problems in the future. Creating an internal interface is almost as easy and more explicit.

Compare to C++

The C++ “friend” keyword is similar to the C# internal keyword. It allows a class or a function to access private members of a class. The difference is it allows access to specific class or function and not all the classes in the same DLL. In my opinion, this is a better solution than the C# internal keyword.

Write Clean Code Without Loops

Shani Fedida — Sun, 30 Aug 2020 17:26:27 +0000

Loops are messy - Photo by Daniel Fazio on Unsplash

A few years ago, I participated in a public workshop organized by Wix. We practiced TDD by implementing the Game Of Life kata with different constraints. One constraint was to write code without loops. I was shocked and stuck, I could not grasp how it is possible. Eventually, I figured it out: map, filter, and reduce! I knew this concept before but it wasn’t part of my daily programming (back then I wrote in C++). When I returned to C#, I started using this concept more and more and understood its benefits.

In this post, I show how to write clean code by replacing foreach loops with map, filter, and reduce. This approach is relevant to all languages.

TL;DR

Declarative code written with map, filter, and reduce is more readable and concise than imperative code written with foreach loops.
Reduce boilerplate code and avoid stupid bugs.
Achieve clean code by separating between the logic of which items to operate on and the operation itself.
Code is better in terms of the Single Responsibility Principle

What Are Map, Filter & Reduce?

Map, Filter and Reduce are functions that work on collections. They receive a collection and a function and return a new collection based on the result of the function.

Map — maps each object in a collection into a new object. It returns a new collection with the same size as the given collection.
Filter — filters a collection according to a given condition. The new collection includes only the elements that pass the condition.
Reduce — reduces a collection down to a single value.

// Detect dark theme var iframe = document.getElementById('tweet-741089564329054208-974'); if (document.body.className.includes('dark-theme')) { iframe.src = "https://platform.twitter.com/embed/Tweet.html?id=741089564329054208&theme=dark" }

I wrote the code examples with C# LINQ and simplified them for the post. In real-life systems the code is not clearly divided into functions, instead, most of the code is inside the loop.

Replace foreach with map

Sometimes we see code with this structure:

Now, let’s rewrite it using LINQ Select which is the LINQ version of map :

* I call ToList() because LINQ is lazy.

This code is shorter — a single line!

Replace if with filter

Sometimes we see code with these structures:

The loops contain two logics:

Which items to operate on.
The operation itself.

We can replace if with LINQ Where, which is the LINQ version of filter. When we replace if (someCondition) continue; with Where we write the negative form of the condition.

The two logics are now separated. Each logic has a single responsibility.

Replace Boilerplate Code with Any

Sometimes we see code with these structures:

These are patterns for “return true if at least one of the items returns true for a given condition” aka the Any function. LINQ Any determines whether any element of a sequence exists or satisfies a condition. Any is a specific form of a reduce function.

Even though we are used to writing these boilerplate patterns, there is always a chance to cause bugs by doing a wrong initialization or confusing between the AND operator and the OR operator.

Let’s rewrite the code by using LINQ Any:

This code is clearer, shorter (single line!), and emphasizes the code’s meaning. It is also more efficient, Any stops as soon as the result can be determined.

Replace Boilerplate Code with All

Sometimes we see code with these structures:

These patterns mean “return true if ALL the items return true for a given condition” aka the All function. LINQ All Determines whether all elements of a sequence satisfy a condition. All is a specific form of a reduce function.

Similar to Any, there is a chance to miswrite this boilerplate code and to cause bugs.

Let’s rewrite the code by using LINQ All.

This code is clearer, shorter (single line!), and emphasizes the code’s meaning.

Code Evolution

In this section, I show how writing code with foreach loops can lead to spaghetti code while writing code with map, filter, and reduce results in clear, readable, and maintainable code.

Let’s imagine we are building a fun system for organizing fun days. 💃 🎉 🍻
Each employee should receive a welcome email for the fun day they registered to. Here is the code:

Now we discover that some people forgot to register. They were unaware of the fun days 😲. What should we do? Let’s send them a reminder email to register for a fun day.

Sending a reminder also operates only on current employees. The current code already finds the fun day for each employee, so this functionality is quite simple to add.

Great! Now people will not miss out on the fun. 😃

Now we discover that some people registered for a fun day but forgot to register for the optional activities. Let’s send them a reminder to register for the optional activities.

The current code already finds the fun day for each employee, so let’s use it to add the new functionality.

Yammi 😋 we got spaghetti… code! 🤦‍♀

Computer vector created by freepik — www.freepik.com

The problems in the above code are:

Many logics mixed together aka violates the Single Responsibility Principle.
In each row, employee passed different filters so it is hard to grasp which filters it passed.
Quite nested code (4 nesting levels)

Let’s Try Again — Rewrite 💣💣💣

Now we start from scratch, writing the code with LINQ.

Let’s rewrite the first feature — welcome email.

The original code:

Rewrite with LINQ:

What are the differences?

In both implementations SendWelcomeEmail operates only on employes who are registered for a fun day. In the LINQ implementation, I separated the logic of whom to operates and the operation itself.
I separated the filtering of current employees from the employes' loop. The filtering is done in a separate function called GetCurrentEmployees which emphasizes the code's meaning. I replaced if (IsPastEmployee(employee)) continue; and if (IsFutureEmployee(employee)) continue; with Where.
Instead of searching the fun day inside the employes' loop, I created a dictionary between an employee and a fun day called employee2Funday. I replaced if (funday.IsEmployeeRegistered(employee.Id)) with employee2Funday.Where.
We assume that an employee is registered to a single fun day. The original code implicitly chooses the first fun day by using the break statement. The new implementation uses LINQ FirstOrDefault which is more explicit. FirstOrDefault is a specific form of a reduce function.

Now let's implement the next feature — a reminder email to register for a fun day for employees who didn’t register for any fun day.

The original code:

Rewrite with LINQ:

What are the differences?

I separated the logic of whom to operates and the operation itself. The function SendUnregisteredToFunday operates only on employes who are unregistered for any fun day.
In the original code, the two functionalities are tangled together. In the rewritten code each functionality has its own method. This is possible because the data this functionality operates on, aka the employees and whether they are registered to a fun day, exists outside of the first functionality implementation.

Now let's implement the last feature — a reminder email for registering to the optional activities.

The original code:

Rewrite with LINQ:

What are the differences?

Again, I separated between the logic of whom to operates and the operation itself. The function SendEmailAboutOptionalActivities operates only on employees who are registered for a fun day but unregistered to any optional activity.
In the original code, the functionalities are tangled together. In the rewritten code each functionality has its own method. This is possible because the data this functionality operates on, aka the employees and whether they are registered to a fun day, exists outside of the other functionalities implementation.
IsUnregisteredToOptionalActivities is implemented with Where and Any instead of boilerplate code. It is shorter and clearer.

This paradigm is part of the functional programming paradigm which has some concepts that help write clean, readable, and less error-prone code. Some of these concepts are immutable classes, avoid variables reassignment, and avoid shared state.

I currently write code in Scala and I barely see foreach loops. Out of curiosity, I gathered some statistics from the codebase I work on: map has ~3500 occurrences, filter has ~3000 occurrences, and foreach has only ~500 occurrences.

Why I Love TDD

Shani Fedida — Fri, 22 May 2020 19:02:02 +0000

In this post, I show the day to day differences between TDD (Test Driven Development) and development without tests or write tests after the feature is complete (TLD) .

TL;DR

TDD takes less time than non-TDD approaches because it reduces manual testing, which takes a lot of time.
Tests are a safety net. In TDD, we have a safety net while coding, not only after we finish the development.
TDD is a methodology that helps developers to focus and work in baby steps, those reducing the cognitive load.
TDD encourages clean code.
Life is easier with TDD because we start with a small amount of code, which slowly grows.

Non-TDD Approaches

Development Without Tests

In this approach, we repeatedly:

Choose scenarios for testing.
Write code for those scenarios.
Test from outside and manually examine that the scenario works correctly by, for example, viewing the data in the UI, looking at the results of API calls, and performing DB queries.
Fix the code until the scenario works as planned.

When do we check previous scenarios still work? I guess each developer checks at a different time: when finish to develop the feature, after each scenario, and even by the number of code changes.

Tests Last Development

This approach is similar to development without tests except that we write tests after we developed the feature in order to validate the feature works correctly.

TDD 101

The overall idea is to write code incrementally. We write code for a specific scenario while always validating that other scenarios still work. Also, we care about clean code, but at a separate stage.

For each scenario, we:

Write a test that represents one scenario. We run the test, and it fails* because we didn’t write any code for it to pass(Compilation errors count as failures)
Write the minimal code for the test to pass.
Refactor the code.

*If the test doesn’t fail, it usually means that we wrote more than the minimal code at a previous iteration, or that the test is equivalent to an existing one.

TDD vs. Non-TDD Approaches

Tests Duration

In non-TDD approaches, we test our code manually by reproducing the same flow over and over again. Reproduction of the flow can include: application loading, login, get to the relevant screen and press a button. The feedback is slow and requires us to remember a sequence of actions.

In TDD, we run all the tests with a press of a button. A unit test usually takes a few milliseconds; an integration test usually takes a few seconds.

TDD takes less time than writing tests in the end because the outcome is the same, but while coding, we still need to test somehow, so we perform manual testing. I believe that TDD also takes less time than develop without tests because the manual tests take more time than write tests.

TDD gives us a simpler and faster way to test our code.

Tests Are a Safety Net

We tested a few scenarios, and we are now implementing the next one. We changed the code, so we should retest the existing scenarios.

In the Non-TDD approach, we test manually. Manually testing takes time, and it’s a repetitive and precise task — not something we humans excel at. In the best case, we documented the scenarios; in the worst case, they are inside our head. What are the chances we really retest all scenarios? Even if we documented the scenarios, do we really retest them all? Or do we skip some because the process is long and repetitive? (I would even say boring).

In TDD, with a press of a button, we test all existing scenarios after each code change.

Focus — Less Context Switch

Focus VS Cognitive Overload

TDD is a methodology that defines a work order. Each task has its time: write a test, write production code, and refactor.

In non-TDD approaches, everyone has their methodology, even if they are not aware of it.

Focusing on a specific scenario and the three stages create a process of small and defined steps — Baby steps 👶. This kind of process creates focus and lowers the cognitive load.

I noticed that for me:

The refactor part helps me to avoid refactoring while coding. More than once, I started to refactor while writing code and ended with messy and broken code. I had to discard the code. 😢
Focusing on a specific scenario helps me ignore other scenarios while implementing it, which reduces my cognitive load.

Clean Code

In non-TDD approaches, every developer decides when to refactor. From my experience, we postpone the refactor to the end of the feature. The problem is that at some point soon, we suffer from the mess we made during the development.

In TDD, there is a dedicated time for refactoring. Because it is at the end of each cycle, we work with more maintainable code while developing.

There is a better chance that working in TDD will produce a cleaner code than the non-TDD approaches, but it depends a lot on the developer.

In TDD the Code Is Written Slower

I know it sounds like a disadvantage, but…

In TDD, we start with a small amount of code, which slowly grows. At every stage in the development, we have less code: less code to read, less code to debug, less code to navigate through =>Life is easier. 😌

I hope I convinced you to try TDD. You are welcome to contact me, and I will be happy to help you adopt it.

DEV Community: Shani Fedida

How to get rid of NullPointerException

So where do all those NULLs come from?

Error handling

Missing optional data for entities

Hierarchical models

Find functions

What are the problems with using nulls?

It will blow up. Eventually…

Hidden errors

Missing non-nullable reference types in C# and Java

So what should we do?

Listen to Uncle Bob

But how?

Using the option type

Splitting the function into two

Splitting the interface

I am not working alone and not on a greenfield project. What now?

The 4th step in TDD

Is the C# internal keyword a code smell?

What is the internal keyword?

Why we need the internal keyword?

Let's see some examples

But we don’t have public classes we use interfaces…

Why not use the internal keyword?

My Conclusion

Compare to C++

Further Reading

Write Clean Code Without Loops

TL;DR

What Are Map, Filter & Reduce?

Replace foreach with map

Replace if with filter

Replace Boilerplate Code with Any

Replace Boilerplate Code with All

Code Evolution

The problems in the above code are:

Let’s Try Again — Rewrite 💣💣💣

What are the differences?

What are the differences?

What are the differences?

Why I Love TDD

TL;DR

Non-TDD Approaches

Development Without Tests

Tests Last Development

TDD 101

TDD vs. Non-TDD Approaches

Tests Duration

Tests Are a Safety Net

Focus — Less Context Switch

Clean Code

In TDD the Code Is Written Slower