Washing your code: avoid reassigning variables

Reassigning variables is like changing the past. When we see:
let pizza = toppings: ['salami', 'jalapeños'] >;
We can’t be sure that our pizza will always have salami and jalapeños on it, because:
- the variable can be reassigned with a new value, even a value of another type;
- the value, if it’s an array or an object, can be mutated.
Knowing that both things are possible makes us think, every time we see pizza in the code, which value it has now. That’s a huge and unnecessary cognitive load that we should avoid.
And most of the time we can avoid both. Let’s start with reassigning and come back to mutation in the next chapter.
Don’t reuse variables
Sometimes a variable is reused to store different values:
function getProductsOnSale(category) category = loadCategory(category); category = category.filter(product => product.onSale); return category; >
Here the category variable is used to store a category ID, a list of products in a category, and a list of filtered products. Even types of these values are different. This function isn’t completely hopeless because it’s short, but imagine more code between reassignments.
Also, a new value is reassigned to a function parameter, known as function parameter shadowing. I think it’s no different from regular reassignment, so I’ll treat it the same way.
This case is the easiest to fix: we need to use separate variables for each value:
function getProductsOnSale(categoryId) const products = loadCategory(categoryId); return products.filter(product => product.onSale); >
By doing this we’re making the lifespan of each variable shorter and choosing clearer names, so the code is easier to understand and we’ll need to read less code to find out the current (and now the only) value of each variable.
Incremental computations
Probably the most common use case for reassignment is incremental computations. Consider this example:
true, validateVideoFileAndUrl = () => true, validateVideoURL = () => true —>
const validateVideo = (video) => let errors = ''; if (!validateHeightWidthConsistency(video.videoFiles)) errors = errors + ERROR_MESSAGES.InconsistentWidthHeight;> // Must provide either both a height + width, or neither if (!validateVideoFileAndUrl(video.videoFiles)) errors = errors + ERROR_MESSAGES.InvalidVideoFiles;> // Must have ONE OF either a file or a URL if (!validateVideoURL(video.videoFiles)) errors = errors + ERROR_MESSAGES.InvalidVideoURL;> // Video URL must be a valid link if (!video[INPUT_TYPES.Title]) errors = errors + ERROR_MESSAGES.BlankTitle;> // Title cannot be blank if (!video[INPUT_TYPES.Id].match(ID_PATTERN) !== false) errors = errors + ERROR_MESSAGES.InvalidId;> // ID must be alphanumeric return errors; >;
I’ve shortened the comments a bit, the original code had lines longer than 200 characters. If we have a very big screen, it looks like a pretty table, otherwise like an unreadable mess. Any autoformatting tool, like Prettier, will make an unreadable mess out of it too, so we shouldn’t rely on manual code formatting. It’s also really hard to maintain: if any “column” becomes longer than all existing “columns” after our changes, we have to adjust whitespace for all other “columns”.
Anyway, this code appends an error message to the errors string variable for every failed validation. But now it’s hard to see because the message formatting code is mangled with the validation code. This makes it hard to read and modify. To add another validation, we have to understand and copy the formatting code. Or to print errors as an HTML list, we have to change each line of this function.
Let’s separate validation and formatting:
true, validateVideoFileAndUrl = () => true, validateVideoURL = () => true —>
const VIDEO_VALIDATIONS = [ // Must provide either both a height + width, or neither isValid: video => validateHeightWidthConsistency(video.videoFiles), message: ERROR_MESSAGES.InconsistentWidthHeight >, // Must have ONE OF either a file or a URL isValid: video => validateVideoFileAndUrl(video.videoFiles), message: ERROR_MESSAGES.InvalidVideoFiles >, // Video URL must be a valid link isValid: video => validateVideoURL(video.videoFiles), message: ERROR_MESSAGES.InvalidVideoURL >, // Title cannot be blank isValid: video => !!video[INPUT_TYPES.Title], message: ERROR_MESSAGES.BlankTitle >, // ID must be alphanumeric isValid: video => video[INPUT_TYPES.Id].match(ID_PATTERN) !== null, message: ERROR_MESSAGES.InvalidId > ]; const validateVideo = video => return VIDEO_VALIDATIONS.map(( isValid, message >) => isValid(video) ? undefined : message ).filter(Boolean); >; const printVideoErrors = video => console.log(validateVideo(video).join('\n')); >;
We’ve separated validations, validation logic, and formatting. Flies separately, kebabs separately, as we say in Russia. Each piece of code has a single responsibility and a single reason to change. Validations now are defined declaratively and read like a table, not mixed with conditions and string concatenation. We’ve also changed negative conditions (is invalid?) to positive (is valid?). All this improves the readability and maintainability of the code: it’s easier to see all validations and add new ones because we don’t need to know the implementation details of running validations or formatting.
And now it’s clear that the original code had a bug: there was no space between error messages.
Also now we can swap the formatting function and render errors as an HTML list, for example:
children, FileUpload = () => null, validateVideo = () => [‘Invalid video’] —>
function VideoUploader() const [video, setVideo] = React.useState(); const errors = validateVideo(video); return ( <> value=video> onChange=setVideo> /> errors.length > 0 && ( <> variation="error">Nooooo, upload failed: errors.map(error => ( key=error> as="li" variation="error"> error> ))> > )> > ); >
); expect(c1.textContent).toEqual(‘Nooooo, upload failed:Invalid video’) —>
We can also test each validation separately. Have you noticed that I’ve changed false to null in the last validation? That’s because match() returns null when there’s no match, not false . The original validation always returns true .
I would even inline ERROR_MESSAGES constants unless they are reused somewhere else. They don’t make code easier to read but they make it harder to change because we have to make changes in two places.
const VIDEO_VALIDATIONS = [ // Must provide either both a height + width, or neither isValid: video => validateHeightWidthConsistency(video.videoFiles), message: 'You should provide either both a height and a width, or neither' > ];
Now all the code we need to touch to add, remove, or change validations is contained in the VIDEO_VALIDATIONS array. Keep the code, that’s likely to be changed at the same time, in the same place.
Building complex objects
Another common reason to reassign variables is to build a complex object:
new Intl.DateTimeFormat().format(x) const SORT_DESCENDING = ‘desc’, DATE_FORMAT = ‘YYYY-MM-DD’ const dateRangeFrom = new Date(2023, 1, 4), dateRangeTo = new Date(2023, 1, 14), sortField = ‘id’ const sortDirection = SORT_DESCENDING, query = » —>
let queryValues = sortBy: sortField, orderDesc: sortDirection === SORT_DESCENDING, words: query >; if (dateRangeFrom && dateRangeTo) queryValues = . queryValues, from: format(dateRangeFrom.setHours(0, 0, 0, 0), DATE_FORMAT), to: format(dateRangeTo.setHours(23, 59, 59), DATE_FORMAT) >; >
Here we’re adding from and to properties only when they aren’t empty.
The code would be clearer if we teach our backend to ignore empty values and build the whole object at once:
new Intl.DateTimeFormat().format(x) const SORT_DESCENDING = ‘desc’, DATE_FORMAT = ‘YYYY-MM-DD’ const dateRangeFrom = new Date(2023, 1, 4), dateRangeTo = new Date(2023, 1, 14), sortField = ‘id’ const sortDirection = SORT_DESCENDING, query = » —>
const hasDateRange = dateRangeFrom && dateRangeTo; const queryValues = sortBy: sortField, orderDesc: sortDirection === SORT_DESCENDING, words: query, from: hasDateRange && format(dateRangeFrom.setHours(0, 0, 0, 0), DATE_FORMAT), to: hasDateRange && format(dateRangeTo.setHours(23, 59, 59), DATE_FORMAT) >;
Now, the query object always has the same shape, but some properties can be undefined . The code feels more declarative and it’s easier to understand what it’s building an and see the final shape of this object.
Avoid Pascal style variables
Some people like to define all variables at the beginning of a function. I call this Pascal style, because in Pascal we have to declare all variables at the beginning of a program or a function:
function max(num1, num2: integer): integer; var result: integer; begin if (num1 > num2) then result := num1 else result := num2; max := result; end;
Some people use this style in languages where they don’t have to do it:
let isFreeDelivery; // 50 lines of code if ( [ DELIVERY_METHODS.PIGEON, DELIVERY_METHODS.TRAIN_CONDUCTOR ].includes(deliveryMethod) ) isFreeDelivery = 1; > else isFreeDelivery = 0; > // 30 lines of code submitOrder( products, address, firstName, lastName, deliveryMethod, isFreeDelivery >);
Long variable lifespan makes us scroll a lot to understand the current value of a variable. Possible reassignments make it even worse. If there are 50 lines between a variable declaration and its usage, then it can be reassigned in any of these 50 lines.
We can make code more readable by moving variable declarations as close to their usage as possible and by avoiding reassignments:
const isFreeDelivery = [ DELIVERY_METHODS.PIGEON, DELIVERY_METHODS.TRAIN_CONDUCTOR ].includes(deliveryMethod); submitOrder( products, address, firstName, lastName, deliveryMethod, isFreeDelivery: isFreeDelivery ? 1 : 0 >);
We’ve shortened isFreeDelivery variable lifespan from 100 lines to just 10. Now it’s also clear that its value is the one we assign at the first line.
Don’t mix it with PascalCase though, this naming convention is still in use.
Avoid temporary variables for function return values
When a variable is used to keep a function result, often we can get rid of that variable:
function areEventsValid(events) let isValid = true; events.forEach(event => if (event.fromDate > event.toDate) isValid = false; > >); return isValid; >
Here we’re checking that every event is valid, which would be more clear with the every() array method:
function areEventsValid(events) return events.every(event => event.fromDate event.toDate); >
We’ve also removed a temporary variable, avoided reassignment, and made a condition positive (is valid?), instead of a negative (is invalid?). Positive conditions are usually easier to understand.
For local variables, we can either use a ternary operator:
const handleChangeEstimationHours = event => let estimationHours = event.target.value; if (estimationHours === '' || estimationHours 0) estimationHours = 0; > return estimationHours >; >;
const handleChangeEstimationHours = ( target: value > >) => const estimationHours = value !== '' && value >= 0 ? value : 0; return estimationHours >; >;
Or we can extract code to a function, for example:
let rejectionReasons = getAllRejectionReasons(); if (isAdminUser) rejectionReasons = rejectionReasons.filter( reason => reason.value !== REJECTION_REASONS.HAS_SWEAR_WORDS ); >
const getRejectionReasons = isAdminUser => const rejectionReasons = getAllRejectionReasons(); if (isAdminUser) return rejectionReasons.filter( reason => reason.value !== REJECTION_REASONS.HAS_SWEAR_WORDS ); > return rejectionReasons; >; // --- 8< -- 8< --- const rejectionReasons = getRejectionReasons(isAdminUser);
This is less important. You may argue that moving code to a new function just because of a reassignment isn’t a great idea, and you may be right, so use your own judgment here.
Indeterminate loops
Sometimes having a reassignment is quite okay. Indeterminate loops, the ones where we don’t know the number of iterations in advance, are a good case for reassignments.
Consider this example:
function getStartOfWeek(selectedDay) let startOfWeekDay = selectedDay; while (startOfWeekDay.getDay() !== WEEK_DAY_MONDAY) startOfWeekDay = addDays(startOfWeekDay, -1); > return startOfWeekDay; >
3>).getDay()).toEqual(0) —>
Here we’re finding the start of the current week by moving one day back in a while loop and checking if it’s already Monday or not.
Even if it’s possible to avoid a reassignment here, it will likely make the code less readable. Feel free to try and let me know how it goes though.
Reassignments aren’t pure evil and exterminating them all won’t make our code better. They are more like signs: if we see a reassignment, we should ask ourselves if rewriting the code without it would make it more readable. There’s no right or wrong answer, but if we do use a reassignment, it’s better to isolate it in a small function, where it’s clear what the current value of a variable is.
Help the brain with conventions
In all examples above I’m replacing let with const in variable declarations. This immediately tells the reader that the variable won’t be reassigned. And we can be sure, it won’t: the compiler will yell at us if we try. Every time we see let in the code, we know that this code is likely more complex and needs more brain power to understand.
Another useful convention is using UPPER_CASE names for constants. This tells the reader that this is more of a configuration value, than a result of some computation. The lifespan of such constants is usually large: often the whole module or even the whole codebase, so when we read the code we usually don’t see the constant definition, but we still can be sure that the value never changes. And using such a constant in a function doesn’t make the function not pure.
There’s an important difference between a variable defined with the const keyword and a true constant in JavaScript. The first only tells the compiler and the reader that the variable won’t be reassigned. The second describes the nature of the value as something global and static that never changes at runtime.
Both conventions reduce the cognitive load a little bit and make the code easier to understand.
Unfortunately, JavaScript has no true constants, and mutation is still possible even when we define a variable with the const keyword. We’ll talk about mutations in the next chapter.
Start thinking about:
- Using different variables with meaningful names instead of reusing the same variable for different purposes.
- Separating data from an algorithm to make code more readable and maintainable.
- Building a shape of a complex object in a single place instead of building it piece by piece.
- Declaring variables as close as possible to the place where they are used reduces the lifespan of a variable and makes it easier to understand which value a variable has at a particular moment.
- Extracting a piece of code to a small function to avoid a temporary variable and use a function return value instead.
Read other sample chapters of the book:
- Naming is hard
- Avoid reassigning variables (this post)
- Avoid mutation
- Avoid loops
- Avoid conditions
- Avoid comments
IDEA сплошное подчеркивание серым — что это? [дубликат]

Что означает такое вот подчеркивание?
Отслеживать
задан 10 янв 2019 в 15:44
user224616 user224616
По поводу вашей правки: нет, не «могут изменяться», а именно что изменяются. В показанном вами коде requestLocale тоже «может изменяться», но не подчёркивается, потому что не изменяется
12 янв 2019 в 15:05
Я имел в виду, если if -ы не сработают, то locale не изменится, т.е. именно МОГУТ изменяться, а не изменяются
– user224616
12 янв 2019 в 15:12
Ваша интерпретация интересена, но, подозреваю, для анализатора кода не применима: с его точки зрения, если есть locale = — значит считаем за изменение, даже если завернуть в if (false) и реального изменения никогда не произойдёт — всё равно подчеркнётся (я не проверял, но подозреваю, что так и будет, проверьте на всякий случай)
Notification saying «Reassigned local variable» in Android Studio
My string is getting underlined, with a notification saying «Reassigned local variable» . I have worked out that this happens any time that the string is modified, but I am not sure why this notification is coming up. It doesn’t appear do be causing any errors, and nothing is showing under the «problems» tab, so I’m not sure if it is something I should change or if I should just ignore it. Below is a simplified version of the code.
Canvas canvas; Paint p = new Paint(); String value = "a"; //'value' here becomes underlined if(someCondition()) value += "bcdefghijklmnop"; //'value' here becomes underlined canvas.drawText(value, 50, 50, p);
asked Dec 22, 2022 at 22:24
29 1 1 silver badge 7 7 bronze badges
Whats the question? Do you want to avoid re-assigning the local variable or remove the notification?
Dec 22, 2022 at 22:29
1 Answer 1
It is just a notification, probably because sometimes reassigned local variables can cause problems for later operations. In your case it is totally legit to reassign, just be aware that it will create several new objects.
If you would like to prevent this, you could work with a StringBuilder here that you convert to a String whenever necessary, but this also creates a new String object, so if you don’t use this in a loop usually it is not necessary to change anything (and for the string manipulation in a loop you probably will get a warning, a stronger sign for a potential problem in your code than this actual notification).
answered Dec 22, 2022 at 22:55
cyberbrain cyberbrain
3,834 1 1 gold badge 13 13 silver badges 24 24 bronze badges
- java
- android-studio
-
Featured on Meta
Related
Hot Network Questions
Subscribe to RSS
Question feed
To subscribe to this RSS feed, copy and paste this URL into your RSS reader.
Site design / logo © 2024 Stack Exchange Inc; user contributions licensed under CC BY-SA . rev 2024.1.26.3951
Reassign parameter to local variable
Is this just some cargo-cult practice that some programming teachers pass on, or are there coding style guidelines that actually recommend it? The only justification I can think of for this would be in languages that pass parameters by reference, if the function needs to reassign the variable without affecting the caller’s variable. Is this a legacy of instructors who grew up learning Fortran, which passes all parameters by reference? Most other languages with by-reference parameters require them to be declared explictly, as in C++’s type ¶meter declaration, and in these cases it’s usually desirable to modify the caller’s variable. This style of coding for variables that are reassigned is discussed in Is there a reason to not modify values of parameters passed by value?. But I also see this in functions that only read the variable. For instance, this question has a function like this:
function say(messages = []) < let alphabets = []; let textLines = messages console.log(textLines.length) textLines.forEach((line, i) =>< if (i < textLines.length - 1) line.text += " "; // more code >); >);
It could just as easily be written with textLines as the parameter name.
asked Oct 5, 2022 at 18:46
324 2 2 silver badges 5 5 bronze badges
2 Answers 2
It will happen if you want a different name for the parameter at the caller side and for the argument on the callee side. The developer making the call and the developer implementing the function can have different views of things.
Another situation are parameters being references and you don’t want to unintentionally modify the original, or read-only, or you want both a modified value and the original value.
So there are cases where it makes sense and cases where it doesn’t. If you see it, probably not worth changing.
answered Oct 6, 2022 at 10:27
gnasher729 gnasher729
44k 4 4 gold badges 61 61 silver badges 125 125 bronze badges
But we’re talking about exercise code, not code implementing an API with pre-ordained parameter names. The programmer gets to decide what the variables are named.
Oct 6, 2022 at 13:41
Is this just some cargo-cult practice that some programming teachers pass on,
There are millions of teachers all over the world, so how shall we know?
or are there coding style guidelines that actually recommend it?
I have never seen one, but the situation is not different than the former: there are too many coding standards all over the world to answer this.
Hence I think it is impossible to answer this generically «for all questions on SO which use this style» in a sensible manner. One has to look at each individual case, review the code or ask the authors for their reasons. So please do yourself a favor and stop asking questions which wrongly assume the existence of some overgeneralizated answer.
In your specific example, however, I guess the two different names simply reflect the different points of view of the interface vs. implementation:
- on the caller’s side, the function signature is just say(messages) . The name messages just gives the caller a hint what the parameter should contain.
- from the implementors’s point of view, those messages may be interpreted as text lines, so they will be given a different name. The primary context is here the internal agorithm. This is hidden from the caller, since it is an implementation detail.
So introducing a alias name can make sense in certain situations. However, for other situations there might be different reasons, or it is quite unneccessary — one has to look at each individual case.