Skip to content

Lodash methods and helper functions #149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alexanderroidl wants to merge 6 commits into from
Closed

Lodash methods and helper functions #149

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d2abdc8
refactor: utilize lodash methods
alexanderroidl Jul 26, 2025
da3696c
refactor: switch helper function for truthyness check
alexanderroidl Jul 26, 2025
ea57830
revert: accidential format
alexanderroidl Jul 26, 2025
3884295
fix: add missing negate
alexanderroidl Jul 26, 2025
3188bf2
chore: user input validity check
alexanderroidl Jul 31, 2025
2916394
Merge branch 'master' into refactor/lodash-and-helper-functions
alexanderroidl Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions lib/api/routes/userRoute.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ function checkIfAnyAdminAfterRemovingUser(userIdToBeRemoved, allUser) {
function checkIfUserToBeRemovedIsLoggedIn(userIdToBeRemoved, req) {
return req.session.currentUser === userIdToBeRemoved;
}
const nullOrEmpty = (str) => str == null || str.length === 0;
userRouter.get('/', async (req, res) => {
res.body = userStorage.getUsers(false);
res.send();
Expand Down Expand Up @@ -42,7 +41,6 @@ userRouter.delete('/', async (req, res) => {
res.send();
});
userRouter.post('/', async (req, res) => {

if(config.demoMode){
res.send(new Error('In demo mode, it is not allowed to change or add user.'));
return;
Expand All @@ -53,7 +51,7 @@ userRouter.post('/', async (req, res) => {
res.send(new Error('Passwords does not match'));
return;
}
if (nullOrEmpty(username) || nullOrEmpty(password) || nullOrEmpty(password2)) {
if (!username || !password || !password2) {
Copy link
Owner

@orangecoding orangecoding Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the previous pr. your check is not the same as mine.

let a = 0;
nullOrEmpty(a); //false
!a; //true

Copy link
Owner

@orangecoding orangecoding Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my case I want to explicitly check for null or empty

Copy link
Contributor Author

@alexanderroidl alexanderroidl Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I changed it back to the same logic but slimer:

const nullOrEmpty = (str) => str == null || str.length === 0;

is the same as

const nullOrEmpty = (str) => Boolean(str?.length);

as the if-block will convert everything inside it's brackets to a boolean value for checking, we can inline it:

  if (!username?.length || !password?.length || !password2?.length) {

Let me know if that works for you.

Copy link
Owner

@orangecoding orangecoding Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that is not true... (Or I'm missing something here):
Your version: const nullOrEmpty = (str) => Boolean(str?.length);
returns false on empty string.

const nullOrEmpty = (str) => Boolean(str?.length);
console.log(nullOrEmpty(''); //false, but should be true

Copy link
Contributor Author

@alexanderroidl alexanderroidl Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I typed it the wrong way, I meant

const nullOrEmpty = (str) => !Boolean(str?.length);

as an if-block implicitly converts statements inside it to a boolean for checking it, we can check for !str?.length directly instead of implementing and calling isNullOrEmpty(str)

res.send(new Error('Username and password are mandatory.'));
return;
}
Expand Down
28 changes: 13 additions & 15 deletions lib/notification/adapter/telegram.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { markdown2Html } from '../../services/markdown.js';
import { getJob } from '../../services/storage/jobStorage.js';
import fetch from 'node-fetch';
import lodash from 'lodash';

const MAX_ENTITIES_PER_CHUNK = 8;
const RATE_LIMIT_INTERVAL = 1010;
/**
Expand All @@ -9,30 +11,26 @@ const RATE_LIMIT_INTERVAL = 1010;
* @param inputArray
* @param perChunk
*/
const arrayChunks = (inputArray, perChunk) =>
inputArray.reduce((all, one, i) => {
const ch = Math.floor(i / perChunk);
all[ch] = [].concat(all[ch] || [], one);
return all;
}, []);
function shorten(str, len = 30) {
return str.length > len ? str.substring(0, len) + '...' : str;
}

export const send = ({ serviceName, newListings, notificationConfig, jobKey }) => {
const { token, chatId } = notificationConfig.find((adapter) => adapter.id === config.id).fields;
const job = getJob(jobKey);
const jobName = job == null ? jobKey : job.name;

// we have to split messages into chunks, because otherwise messages are going to become too big and will fail
const chunks = arrayChunks(newListings, MAX_ENTITIES_PER_CHUNK);
const chunks = lodash.chunk(newListings, MAX_ENTITIES_PER_CHUNK);
const promises = chunks.map((chunk) => {
const messageParagraphs = [];

messageParagraphs.push(`<i>${jobName}</i> (${serviceName}) found <b>${newListings.length}</b> new listings:`);
messageParagraphs.push(...chunk.map(
(o) =>
`<a href='${o.link}'><b>${shorten(o.title.replace(/\*/g, ''), 45).trim()}</b></a>\n` +
[o.address, o.price, o.size].join(' | ')
));
messageParagraphs.push(
...chunk.map((o) => {
const normalizedTitle = o.title.replace(/\*/g, '');
const titleExcerpt = lodash.truncate(normalizedTitle, { length: 45, omission: '…' });

return `<a href='${o.link}'><b>${titleExcerpt}</b></a>\n` + [o.address, o.price, o.size].join(' | ');
}),
);

/**
* This is to not break the rate limit. It is to only send 1 message per second
Expand Down
8 changes: 3 additions & 5 deletions lib/provider/immoscout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,11 @@ async function getListings(url) {
});
}

function nullOrEmpty(val) {
return val == null || val.length === 0;
}
function normalize(o) {
const title = nullOrEmpty(o.title) ? 'NO TITLE FOUND' : o.title.replace('NEU', '');
const address = nullOrEmpty(o.address) ? 'NO ADDRESS FOUND' : (o.address || '').replace(/\(.*\),.*$/, '').trim();
const id = buildHash(o.id, o.price);
const title = o.title ? o.title.replace('NEU', '') : 'NO TITLE FOUND';
const address = o.address ? o.address.replace(/\(.*\),.*$/, '').trim() : 'NO ADDRESS FOUND';

return Object.assign(o, { id, title, address });
}
function applyBlacklist(o) {
Expand Down
9 changes: 2 additions & 7 deletions lib/provider/neubauKompass.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,10 @@ import utils, { buildHash } from '../utils.js';

let appliedBlackList = [];

function nullOrEmpty(val) {
return val == null || val.length === 0;
}

function normalize(o) {
const link = nullOrEmpty(o.link)
? 'NO LINK'
: `https://www.neubaukompass.de${o.link.substring(o.link.indexOf('/neubau'))}`;
const id = buildHash(o.link, o.price);
const link = o.link ? `https://www.neubaukompass.de${o.link.substring(o.link.indexOf('/neubau'))}` : 'NO LINK';

return Object.assign(o, { id, link });
}

Expand Down
8 changes: 1 addition & 7 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ function isOneOf(word, arr) {
return arr.some(item => lowerWord.indexOf(item.toLowerCase()) !== -1);
}

function nullOrEmpty(val) {
return val == null || val.length === 0;
}

function timeStringToMs(timeString, now) {
const d = new Date(now);
const parts = timeString.split(':');
Expand All @@ -29,7 +25,7 @@ function timeStringToMs(timeString, now) {

function duringWorkingHoursOrNotSet(config, now) {
const {workingHours} = config;
if (workingHours == null || nullOrEmpty(workingHours.from) || nullOrEmpty(workingHours.to)) {
if (!workingHours?.from || !workingHours?.to) {
Copy link
Owner

@orangecoding orangecoding Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd consider checking for null or empty by doing this truthy/falsy check relatively unsafe for reason mentioned above. WDYT?

Copy link
Contributor Author

@alexanderroidl alexanderroidl Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're making a good point generally!

In this case I think we're still good though as the check will only be true when workingHours

  • is an object (=neither null, nor undefined)
  • has a from property
  • the content of that property is truthy

Which would actually make the check more complex and safer here.

> console.log(undefined?.test)
undefined
> console.log(Boolean(undefined?.test))
false
> console.log(""?.test)
undefined
> console.log(Boolean(""?.test))
false
> console.log(''?.test23abc)
undefined
> console.log(Boolean(''?.test23abc))
false

return true;
}
const toDate = timeStringToMs(workingHours.to, now);
Expand Down Expand Up @@ -74,14 +70,12 @@ await refreshConfig();

export {isOneOf};
export {inDevMode};
export {nullOrEmpty};
export {duringWorkingHoursOrNotSet};
export {getDirName};
export {config};
export {buildHash};
export default {
isOneOf,
nullOrEmpty,
duringWorkingHoursOrNotSet,
getDirName,
config,
Expand Down
2 changes: 1 addition & 1 deletion test/services/immoscout/immoscout-web-translater.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { convertWebToMobile } from '../../../lib/services/immoscout/immoscout-web-translater.js';
import { convertWebToMobile } from '../../../lib/services/immoscout/immoscout-web-translator.js';
import { expect } from 'chai';
import { readFile } from 'fs/promises';

Expand Down
11 changes: 3 additions & 8 deletions ui/src/views/generalSettings/GeneralSettings.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ const GeneralSettings = function GeneralSettings() {
init();
}, [settings]);

const nullOrEmpty = (val) => val == null || val.length === 0;

const throwMessage = (message, type) => {
if (type === 'error') {
Toast.error(message);
Expand All @@ -80,18 +78,15 @@ const GeneralSettings = function GeneralSettings() {
};

const onStore = async () => {
if (nullOrEmpty(interval)) {
if (!interval) {
throwMessage('Interval may not be empty.', 'error');
return;
}
if (nullOrEmpty(port)) {
if (!port) {
throwMessage('Port may not be empty.', 'error');
return;
}
if (
(!nullOrEmpty(workingHourFrom) && nullOrEmpty(workingHourTo)) ||
(nullOrEmpty(workingHourFrom) && !nullOrEmpty(workingHourTo))
) {
if ((workingHourFrom && !workingHourTo) || (!workingHourFrom && workingHourTo)) {
throwMessage('Working hours to and from must be set if either to or from has been set before.', 'error');
return;
}
Expand Down
Loading