Range validator for multiple types I have an existing overloaded set of functions that I cannot change and need to be tested:

object Validator {
    fun validateRange(min: Int, max: Int): Boolean = true
    fun validateRange(min: Long, max: Long): Boolean = true
    fun validateRange(min: Double, max: Double): Boolean = true
    fun validateRange(min: String, max: String): Boolean = true

    fun validateRangeInput(min: Int, max: Int, input: Int): Boolean = true
    fun validateRangeInput(min: Long, max: Long, input: Long): Boolean = true
    fun validateRangeInput(min: Double, max: Double, input: Double): Boolean = true
    fun validateRangeInput(min: String, max: String, input: String): Boolean = true
}

The test data for validateRange() contain each of the various types. Here is a sample of the input data:

val inputs = listOf(
        Pair("1", "1"),
        Pair(1, 1),
        Pair(1L, 1L),
        Pair(1.0, 1.0)
)

Of course, I cannot simply do this:

inputs.forEach { (min, max) -> Validator.validateRange(min, max) }

My current solution implements a helper using generics and a when block with the is operator and smart casting:

fun <T> validateRangeHelper(min: T, max: T): Boolean {
    return when {
        min is Int && max is Int -> Validator.validateRange(min, max)
        min is Long && max is Long -> Validator.validateRange(min, max)
        min is Double && max is Double -> Validator.validateRange(min, max)
        min is String && max is String -> Validator.validateRange(min, max)
        else -> throw UnsupportedOperationException()
    }
}

inputs.forEach { (a, b) -> validateRangeHelper(a, b) }

This approach grows cumbersome with each additional argument, such as for the validateRangeInput() functions.

fun <T> validateRangeHelper(min: T, max: T, input: T): Boolean {
    return when {
        min is Int && max is Int && input is Int -> Validator.validateRangeInput(min, max, input)
        min is Long && max is Long && input is Long -> Validator.validateRangeInput(min, max, input)
        min is Double && max is Double && input is Double -> Validator.validateRangeInput(min, max, input)
        min is String && max is String && input is String -> Validator.validateRangeInput(min, max, input)
        else -> throw UnsupportedOperationException()
    }
}

Is there a way I can improve the helper or avoid it all together without changing Validator?

Definitely not one of my proudest answers...
You can delegate the work of casting to another function

fun <T> Pair<*,*>.validate2(lambda: (T, T)->Boolean) = lambda(first as T, second as T)

In your validateHelperclass you can call this function:

fun <T> validateRangeHelper(pair: Pair<T,T>): Boolean = when(pair.first) {
    is Int -> pair.validate3<Int>(Validator::validateRange)
    is Double -> pair.validate3<Double>(Validator::validateRange)
    is Long -> pair.validate3<Long>(Validator::validateRange)
    is String -> pair.validate3<String>(Validator::validateRange)
    else -> throw UnsupportedOperationException()
}

(I called it validate2, as I created a custom class which has more than 2 parameters, such that I could test both the methods, based on  if i called validate2 or validate3...)

typeunsafe

//just to be able to get three
class TestValues<T : Any>(
    val min: T,
    val max: T,
    val input: T
)

I search the functions with a certain name and parameter count. I extracted it from a function, but of course you can type the name yourself. Note, you have to import kotlin.reflect.KFunction2.

inline fun <reified R : Any> createTester2WithInt(obj: R, f: KFunction2<Int, Int, Boolean>) =
    Tester2(
        obj,
        R::class.findMethodsWithType(f.name, 2)
    )

inline fun <reified R : Any> createTester3WithInt(obj: R, f: KFunction3<Int, Int, Int, Boolean>) =
    Tester3(
        obj,
        R::class.findMethodsWithType(f.name, 3)
    )

open class Tester(
    private val obj: Any,
    private val map: Map<KClass<out Any>, KFunction<Any?>>
) {
    @PublishedApi // means you shouldn't call it by yourself
    internal inline fun <reified T, reified S : Any> call(vararg args: S, clazz: KClass<out S>): T {
        val method = map[clazz] ?: throw java.lang.UnsupportedOperationException()
        return method.call(obj, *args) as T
    }
}

class Tester2(obj: Any, map: Map<KClass<out Any>, KFunction<Any?>>) : Tester(obj, map) {
    inline fun <reified T, reified S : Any> test(first: S, second: S): T = call(first, second, clazz = first::class)
}

class Tester3(obj: Any, map: Map<KClass<out Any>, KFunction<Any?>>) : Tester(obj, map) {
    inline fun <reified T, reified S : Any> test(first: S, second: S, third: S): T = call(first, second, third, clazz = first::class)
}

fun <R : Any> KClass<out R>.findMethodsWithType(
    name: String,
    parameterCount: Int
) = memberFunctions.asSequence()
    .filter { it.name == name }
    //the returnType is also a parameter
    .filter { it.parameters.size == parameterCount + 1 }
    .filter { it.returnType.jvmErasure == Boolean::class }
    .filter {
        val paramTypes = it.javaMethod!!.parameterTypes
        paramTypes[0] == paramTypes[1]
    }.associateBy { it.javaMethod!!.parameterTypes[0].kotlin }

fun create2Pred(pred: (Any, Any) -> Boolean) = pred

fun main() {
    val validator2 = createTester2WithInt<Validator>(Validator, Validator::validateRange)
    val validator3 = createTester3WithInt<Validator>(Validator, Validator::validateRangeInput)
    val list = listOf(
        TestValues(1, 1, 1),
        TestValues(1.0, 1.0, 1.0),
        TestValues("1", "1", "1")
    ).forEach {
        val a: Boolean = validator2.test(it.min, it.max)
        val b: Boolean = validator3.test(it.min, it.max, it.input)
        println("$a and $b")
    }
}

object Validator {
    fun validateRange(min: Int, max: Int): Boolean = true
    fun validateRange(min: Long, max: Long): Boolean = true
    fun validateRange(min: Double, max: Double): Boolean = true
    fun validateRange(min: String, max: String): Boolean = true

    fun validateRangeInput(min: Int, max: Int, input: Int): Boolean = true
    fun validateRangeInput(min: Long, max: Long, input: Long): Boolean = true
    fun validateRangeInput(min: Double, max: Double, input: Double): Boolean = true
    fun validateRangeInput(min: String, max: String, input: String): Boolean = true
}


Two groups of Buttons but with same class name I have two groups of two buttons (status). If you click on "Complete" or "On Hold", it will change the background color of the button. I am using same class name for all the buttons but each group of buttons have data attribute data-section='Park One' and data-section='Park Two'

jsFiddle

I am using the $.each() function to detect the section, remove all the background and add the background-color on selected button. Is the code well coded? What can be improved?

jQuery:

$(".updateStatus").click(function () {

    var buttonValue = $(this).val();
    var section = $(this).data("section");

    var cssClass = (buttonValue === "complete") ? 'green' : 'orange';

    $('.updateStatus').each(function () {
        if ($(this).data("section") == section) {

            $(this).removeClass("green orange");

            if ($(this).val() == buttonValue) {
                $(this).addClass(cssClass);
            }
        }
    })

});

HTML

Park One

<button class="updateStatus" value="complete" data-section='Park One'>Complete</button>
<button class="updateStatus" value="on hold" data-section='Park One'>On Hold</button>

Park Two

<button class="updateStatus" value="complete" data-section='Park Two'>Complete</button>
<button class="updateStatus" value="on hold" data-section='Park Two'>On Hold</button>

If you must use jQuery, then the simplest way by far is to wrap each section in a div :

Park One

    <button class="updateStatus" value="complete">Complete</button>
    <button class="updateStatus" value="on hold">On Hold</button>

Park Two

    <button class="updateStatus" value="complete">Complete</button>
    <button class="updateStatus" value="on hold">On Hold</button>

This allows you to use .siblings("button") to select the clicked button's counterpart :

$(".updateStatus").click(function () {
    $(this)
        .addClass(($(this).val() === "complete") ? 'green' : 'orange')
        .siblings("button").removeClass("green orange");
});

The .each() loop vanishes.

It's very common to use divs or spans like this, to give your DOM structure.

Updated fiddle


Starting an activity after 1000ms from a splash screen with Runnable and Handler().postDelayed I have implemented a splash screen which is the main activity of my Android app. This splash screen shows an image, and after 1000ms there is an animation to show another activity's layout.

The code I've written is the following:

public class SplashScreen extends AppCompatActivity {

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.splash_screen);
        startTheTransitionAfterTheSplashScreen();
    }

    private void startTheTransitionAfterTheSplashScreen() {
        new Handler().postDelayed(new Runnable() {
            @Override
            public void run() {
                Intent intentSplashScreenToActivityJustAfterSplashScreen = new Intent(SplashScreen.this, ActivityJustAfterSplashScreen.class);
                startActivity(intentSplashScreenToActivityJustAfterSplashScreen);
                overridePendingTransition(R.anim.animation_enter_activity, R.anim.animation_leave_activity);
                finish();
            }
        }, 1000);
    }
}

And for the animations:

<?xml version="1.0" encoding="utf-8"?>
<set xmlns:android="http://schemas.android.com/apk/res/android"
    android:shareInterpolator="false">
    <translate
        android:fromXDelta="100%" android:toXDelta="0%"
        android:fromYDelta="0%" android:toYDelta="0%"
        android:duration="500" />
</set>

<?xml version="1.0" encoding="utf-8"?>
<set xmlns:android="http://schemas.android.com/apk/res/android"
    android:shareInterpolator="false">
    <translate
        android:fromXDelta="0%" android:toXDelta="-100%"
        android:fromYDelta="0%" android:toYDelta="0%"
        android:duration="500" />
</set>

Question

What I would want to ask is especially about the startTheTransitionAfterTheSplashScreen (I think the animations are correctly written): should I actually use a Runnable with a Handler to delay the transition? (in other words: is it suitable for this splash screen, or is there a better way to do that?)

Edit

Implementing K.H.'s answer:

public class SplashScreen extends AppCompatActivity {

    private Handler the_transition_handler;

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.splash_screen);
    }

    @Override
    protected void onStart() {
        super.onStart();
        startTheTransitionAfterTheSplashScreen();
    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        the_transition_handler.removeCallbacksAndMessages(null);
    }

    private void startTheTransitionAfterTheSplashScreen() {
        the_transition_handler = new Handler();
        the_transition_handler.postDelayed(new Runnable() {
            @Override
            public void run() {
                final Intent intentSplashScreenToActivityJustAfterSplashScreen = new Intent(SplashScreen.this, ActivityJustAfterSplashScreen.class);
                startActivity(intentSplashScreenToActivityJustAfterSplashScreen);
                overridePendingTransition(R.anim.animation_enter_activity, R.anim.animation_leave_activity);
                finish();
            }
        }, 1000);
    }
}

I don't like name startTheTransitionAfterTheSplashScreen. Too long, you are already in SplashScreen. Maybe startTransition or startTransitionafter1second.

Using handler is fine, but what if user manages to change orientation during that 1 second? Activity gets destroyed and recreated. Now you have 2 active handlers and I expect ActivityJustAfterSplashScreen will be called 2 times.

I suggest keeping reference to handler so that you can cancel in onDestroy - handler.removeCallbacksAndMessages(null).


Create array of date pairs for next 10 days I wrote the below function which successfully creates pairs of dates in the format I need. However, I'm fairly new to Python and feel there must be a way to make this easier to read. Can this code be improved?

def getNextTenDays():
    dates = []
    arrive = datetime.datetime.now()
    for n in range(1, 11):
        arrive += datetime.timedelta(days=1)
        next_day = arrive + datetime.timedelta(days=1)
        arrive_s = datetime.datetime.strftime(arrive, '%m/%d/%Y')
        next_day_s = datetime.datetime.strftime(next_day, '%m/%d/%Y')
        dates.append({'arrive': arrive_s, 'depart': next_day_s})
    return dates

Rather that continuing to append to arrive, just use the n offset from 'today'.

You can also use call strftime from a datetime object directly, like so:

def getNextTenDays():
    dates = []
    today = datetime.datetime.now()
    for n in range(1, 11):
        arrive = today + datetime.timedelta(days=n)
        next_day = today + datetime.timedelta(days=n+1)
        arrive_s = arrive.strftime('%m/%d/%Y')
        next_day_s = next_day.strftime('%m/%d/%Y')
        dates.append({'arrive': arrive_s, 'depart': next_day_s})
    return dates

If you are just going to return the strings, and throw away the actual date times you can do that in one go like so:

def getNextTenDays():
    dates = []
    today = datetime.datetime.now()
    for n in range(1, 11):
        dates.append({
            'arrive': (today + datetime.timedelta(days=n)).strftime('%m/%d/%Y'),
            'depart': (today + datetime.timedelta(days=n+1)).strftime('%m/%d/%Y'),
        })
    return dates

For here its a simple step to making it a list comprehension:

def getNextTenDays():
    today = datetime.datetime.now()
    return [
        {
            'arrive': (today + datetime.timedelta(days=n)).strftime('%m/%d/%Y'),
            'depart': (today + datetime.timedelta(days=n+1)).strftime('%m/%d/%Y'),
        }
        for n in range(1,11)
    ]


Python program to solve the "rod-cutting problem" An assignment at school required me to write a program for this task:

  In the rod-cutting problem, we are given a rod of length n inches and
  a table of prices p[i] for i = 1, 2, …, n. Here p[i] is the price of a
  rod of length iinches. We have to find the optimal way of cutting the
  rod so that maximum revenue can be generated by selling the pieces.

Here is my solution to this task (in Python):

def cut_rod(p, n):
    """
    Take a list p of prices and the rod length n and return lists r and s.
    r[i] is the maximum revenue that you can get and s[i] is the length of the
    first piece to cut from a rod of length i.
    """
    # r[i] is the maximum revenue for rod length i
    # r[i] = -1 means that r[i] has not been calculated yet
    r = [-1]*(n + 1)

    # s[i] is the length of the initial cut needed for rod length i
    # s[0] is not needed
    s = [-1]*(n + 1)

    cut_rod_helper(p, n, r, s)

    return r, s

def cut_rod_helper(p, n, r, s):
    """
    Take a list p of prices, the rod length n, a list r of maximum revenues
    and a list s of initial cuts and return the maximum revenue that you can get
    from a rod of length n.

    Also, populate r and s based on which subproblems need to be solved.
    """
    if r[n] >= 0:
        return r[n]

    if n == 0:
        q = 0
    else:
        q = -1
        for i in range(1, n + 1):
            temp = p[i] + cut_rod_helper(p, n - i, r, s)
            if q < temp:
                q = temp
                s[n] = i
    r[n] = q

    return q

n = int(input('Enter the length of the rod in inches: '))

# p[i] is the price of a rod of length i
# p[0] is not needed, so it is set to None
p = [None]
for i in range(1, n + 1):
    price = input('Enter the price of a rod of length {} in: '.format(i))
    p.append(int(price))

r, s = cut_rod(p, n)
print('The maximum revenue that can be obtained:', r[n])
print('The rod needs to be cut into length(s) of ', end='')
while n > 0:
    print(s[n], end=' ')
    n -= s[n]

where s[n] gives us the length of the first piece, s[n – s[n]] gives us the length of the second piece and so on.

Here is an example output -

Enter the length of the rod in inches: 3
Enter the price of a rod of length 1 in: 3
Enter the price of a rod of length 2 in: 7
Enter the price of a rod of length 3 in: 2
The maximum revenue that can be obtained: 10
The rod needs to be cut into length(s) of 1 2

Times taken for each function -

%timeit cut_rod(p, n)
7.65 µs ± 71.7 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

%timeit cut_rod_helper(p ,n, r, s)
266 ns ± 3.97 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

So, I would like to know whether I could make this program shorter and more efficient.

Any help would be highly appreciated.

First of all, it's really a bad idea to change the value of the input parameter when this change has an outer effect in some other place in your code.

Also, You can get rid of recursive function calls, that makes your code so much faster.
So let's start from cut_rod_helper function:

def cut_rod_helper(p, n, r, s):
    q = 0
    for i in range(1, n + 1):
        temp = p[i] + r[n - i]
        if q < temp:
            q = temp
            s[n] = i
    return q

I removed comments for more clear view here. But it's good to have them in your code.

What I did here is removing recursion. I removed first if statement from your code because we don't do recursive call and don't need end of recursion statement. As you can see at the middle of the function, we get r value from that list directly.

Second change here is I removed second if statement in your code because we don't need this (At least when we only have prices more than zero). Fewer conditions mean better code. Because with fewer if and else, the code is more straightforward and less error-prone. Also in many cases, the if-less code is faster. So always try to minimize the number of if conditions in your code.

We can simply set q value to zero initially. If n be 0, range of for loop will be (1,1), when start and stop values of a for loop are same, that for will not execute. So you don't need to consider a different condition for that.

And the biggest trick is in the third line of the function. We really don't need recursive call. If we start calling helper function from 0 to n index of r, we always have previous values in the r and we can access them directly.

Now let's go to the primary function:

def cut_rod(p, n):
    r = [-1]*(n + 1)
    s = [-1]*(n + 1)
    r[0] = 0
    for i in range(1, n + 1):
        r[i] = cut_rod_hezerolper(p, i, r, s)
    return r, s

We have only two changes here. First, I have been set index 0 of r to zero. It's our initial state and is clear from problem description. length = 0 so price = 0!

Then, for avoiding the change of input parameters, We are looping through the r and set value of each index of it. Because for calculating ith value of r, we only need access to i - 1 values of it, and we have been set initial value r[0] and start our loop from i = 1, now our code works without any problem.

I did not a full performance test, but with few tries, the second version is about 3 times faster than the first version of your code.

In the end, I did not change variable names, because I think that may distract you. But I think you should change your variable names. s, r, q and ... are meaningless. If a variable is a list contains prices, you should name it something like prices_list. If you do that, you don't need many lines of those comments and everyone can find out what you did in your code at first glance. Spending some seconds on naming things in programming, saving minutes and hours later.

I hope this helps you.


Caml light and finding the prime factors I wrote the following Caml light code to find prime factors of a number (part of an early Project Euler question).

  Largest prime factor

  The prime factors of 13195 are 5, 7, 13 and 29.

  What is the largest prime factor of the number 600851475143?

let rec find_factors target check =
if (check > (int_of_float (sqrt (float_of_int target)))) then [] else
        (find_factors target (check+1))@if target mod check == 0 then
[ check; (target/check)] else[] ;;

let display n =
print_int n;print_newline();;

do_list display (find_factors 10 1);
print_newline();;

I'm very new to functional programming and I suspect I'm all over the place - if anyone would like to give me some tips that would be ideal. :)

I guess that I should not be giving you an answer, but guide you. So, one general remark that I have: stylistically, it would be preferable to see you start with

let largest_factor n =
    let rec aux_factor d n =
        ...
    in
        # some expression involving to aux_factor

The idea is that your solution function should be an expression of your problem and nothing else, so largest_factor number, not largest_factor number what_is_that.

Note that I am talking OCaml here, not sure if the pseudosample I wrote is cromulent Caml Light.


Deleting object properties if they are not found in another object. (javascript) Let's take these two objects as an exemple:

let objA = {
    key1 : "whatever",
    key2 : "whatever",
    key3 : "whatever",
    key4 : "whatever",
    key5 : {
        subKey1: "whatever",
        subKey2: "whatever"
    }
}
let objB = {
    key1 : "whatever",
    key2 : "whatever",
    key3 : "whatever",
    key5 : {
        subKey1: "whatever"
    }
}

I want to delete all the properties of objA if they are not present in objB. The values of the properties do not matter, only the keys themselves. The challenge, of course, is for it to work with any object, no matter how deeply nested they can be.

I came up with this solution based on this answer to a similar question.

let objA = {
    key1 : "whatever",
    key2 : "whatever",
    key3 : "whatever",
    key4 : "whatever",
    key5 : {
        subKey1: "whatever",
        subKey2: "whatever"
    }
}
let objB = {
    key1 : "whatever",
    key2 : "whatever",
    key3 : "whatever",
    key5 : {
        subKey1: "whatever"
    }
}

const deepSameKeys = (o1, o2) => {
    const o1keys = Object.keys(o1).sort();

    return o1keys.every(key => {
        const v1 = o1[key];
        const v2 = o2[key];

        let goDeeper = true;
        if (o2[key] == undefined) {
            console.log('@found a key to delete: ', key);
            delete o1[key];
            goDeeper = false;
        }
        const t1 = typeof v1;
        return t1 === "object" && goDeeper ? deepSameKeys(v1, v2) : true;
    });
};

let result = deepSameKeys(objA, objB)
console.log(result)
console.log(objA) // no more key4 nor subKey2

So far so good, but I wonder if someone can come up with something better or   a situation where my solution would fail.

Bugs and Problems

There are a few problems with your code

Crash when o1 references null.

Reason typeof null === "object" is true.

null is an object when inspected with typeof however Object.keys(null) will throw the error "Can not convert null to Object" so if you have any properties that are null your code will throw an error.
Crash when both objects have similar cyclic references.

Many objects have references to themselves (are cyclic). Your code does not check if there is a cyclic reference and will continue to recurse until the call stack overflows (which is a thrown error)

General

Always aim to keep the code complexity as low as possible.

The variable goDeeper is not needed. You can return true where you set it to false
I do not see why you would need the keys sorted. Sorts are expensive operations, you should always ask yourself "Do I need to sort?" before using it.
The function deepSameKeys will always return true so why return a value at all?
It is unclear if you want to remove undefined properties, or all properties that evaluate to undefined. As you have it you remove all that evaluate to undefined.

I.E. the object {a: undefined} contains a property named a while {} does not contain the property a yet both will evaluate a as undefined.

You can use the in operator which will eval to true for the first object and false for the second. Note this also checks the prototype chain so you may want to use Object.hasOwnProperty if this will be an issue
The naming is very poor, not indecipherable but could be a lot better.
When using recursive functions you should try to avoid using iterators that use anonymous functions EG Array.every

Most recursive functions are just an alternative way to implement a stack to hold the current item's state as you search deeper.

This state is held in the function context (variables scoped to the function)
and stored on the JS heap.

Defining a function inside that context means that a second copy of the state is created as a closure, and also another context for the anonymous function.

The additional state per item is a lot of unneeded overhead (and halves the max depth of recursion).

If you use a for, while, or do loop you avoid the overhead .

Solutions

Bug 1 the solution is to change the lines...

 const t1 = typeof v1;
 return t1 === "object" && goDeeper ? deepSameKeys(v1, v2) : true;

...to include a check for null and have the variable store a bool.

 const isObject = v1 !== null && typeof v1 === "object";
 return isObject ? deepSameKeys(v1, v2) : true;

Note I have removed goDeeper

Bug 2 There are a few options here.

If the objects are parsed directly from JSON files then they can not be cyclic and the is no danger of error. This can also be an unenforced condition of the call (no cyclic object) and let the caller deal with the error is the breach the condition.
Limit the recursion depth. JavaScript's Array.map function uses this method to protect against cyclic references and simply counts the depth down per call and the up on exit. It will not go deeper is this count is <= 0
Track objects recursed using a Set. On each call check if the current object is in the Set, if it is return, if not then put it into the set and process the object.

Rewrite

I am assuming you only want to delete missing properties not properties referencing undefined and the return is not needed.

There are a few versions. After the first I have reduced the variable name lengths to keep the code compact for CR snippet container.

Delete undefined properties in reference from dirty. DOES not remove if property references undefined.
Function returns undefined
Using Object.entries to get at the key and value at the same time.

Keeping the same (almost) behavior and cyclic bug.

const removeMissingProps = (dirty, reference) => {
    Object.entries(dirty).forEach(([key, val]) => {
        if (key in reference) {
            val !== null && typeof val === "object" && removeMissingProps(val, reference[key]);
        } else { delete dirty[key] }
        return true;
    });
};

To provide a deeper call stack and avoid double state capture.  Using for loop.

function remProps(dirty, ref) {
    for (const [key, val] of Object.entries(dirty)) {
        if (key in ref) {
            val !== null && typeof val === "object" && remProps(val, ref[key]);
        } else { delete dirty[key] }
    }
};

To protect against call stack overflow by depth count

function remProps(dirty, ref, depth = 10) {
    if (depth > 0) {
        for (const [k, v] of Object.entries(dirty)) {
            if (k in ref) {
                v !== null && typeof v === "object" && remProps(v, ref[k], depth - 1);
            } else { delete dirty[k] }
        }
    }
};

Uses a WeakSet to check for repeated references.

function remProps(dirty, ref) {
    const checked = new WeakSet();
    remProps(dirty, ref);
    function remProps(dirty, ref) {
        if (!checked.has(dirty)) {
            checked.add(dirty);
            for (const [k, v] of Object.entries(dirty)) {
                if (k in ref) {
                    v !== null && typeof v === "object" && remProps(v, ref[k]);
                } else { delete dirty[k] }
            }
        }
    };
};

Update I forgot to add the last one.

Almost all the recursion functions I see are just implementations of stacks. Recursion has two drawbacks, small stack size (limited to the size of the call stack) and the needless capture of unrelated state.

The following example uses an array rather than the call stack. Its behavior is slightly different (more intuitive) from the function above if you have cyclic references.

There can be significant performance gains using stacks.

function remProps(dirty, ref) {
    const push = (v, ref) => !chk.has(v) && (chk.add(v), stack.push([v, ref]));
    const stack = [[dirty, ref]], chk = new WeakSet([dirty]);
    while (stack.length) {
        [dirty, ref] = stack.pop();
        for (const [k, v] of Object.entries(dirty)) {
            k in ref ?
                v !== null && typeof v === "object" && push(v, ref[k]):
                delete dirty[k];
        }
    }
}


Yet another prime number generator The question asked to find all the prime numbers within a particular range. The way I approached this was to loop through each of the numbers within the range and for each number check whether it's a prime. My method for checking prime is to start at 3 and loop till number/2 in hops of 2 (essentially excluding all the even numbers).

Can somebody take a look at the code an tell me how I might be able to better this snippet and what are some of the aspects that I am missing?

public class PrimeBetween{

public static void main(String[] args){
        printAllPrimes(Integer.parseInt(args[0]),Integer.parseInt(args[1]));
}

public static void printAllPrimes(int start,int end){
        for(int i = start;i <= end;i++){
                if(isPrime(i))
                        System.out.println("Print prime:"+i);
        }

}

private static boolean isPrime(int i){
        if(i%2 == 0 && i!=2)
                return false;
        else{
                if(i == 1) return false;
                for(int p=3;p<=i/2;p+=2){
                        if(i%p == 0)
                                return false;
                }
                return true;
        }

}

}

Well, for starters, it is a proven mathematical fact that if a number is prime, it is not divisible by any prime number that is less than its square root. In your inner loop you go up to i/2 and you can change this to Math.floor(Math.sqrt(i)).

Because you are checking for a range of numbers, you would probably benefit from a cache of primes, rather than testing every odd number less than the square root.

Something like this:

package mypackage;

import java.util.ArrayList;

/**
 * Checks for primeness using a cache.
 */
public class PrimeCache {

    private static ArrayList<Integer> cache = null;

    static {
        cache = new ArrayList<Integer>();
        cache.add(2);
    }

    public static boolean isPrime(int i) {
        if (i == 1) return false; // by definition

        int limit = (int) Math.floor(Math.sqrt(i));
        populateCache(limit);

        return doTest(i, limit);
    }

    private static void populateCache(int limit) {
        int start = cache.get(cache.size() - 1) + 1;
        for (int i = start; i <= limit; i++) {
            if (simpleTest(i)) cache.add(i);
        }
    }

    private static boolean simpleTest(int i) {
        int limit = (int) Math.floor(Math.sqrt(i));

        return doTest(i, limit);
    }

    private static boolean doTest(int i, int limit) {
        int counter = 0;
        while (counter < cache.size() && cache.get(counter) <= limit) {
            if (i % cache.get(counter) == 0) return false;
            counter++;
        }

        return true;
    }
}


How do I refactor this fetch request? I have this code that fetches data from a 3rd party api and sends it to my backend to be processed. The 3rd party api needs to target the user's localhost address so it has to come from the front end. I have the code below which works perfectly fine but it looks messy, I'm sure there is a better way to write it. Any suggestions?
I am using fetch for the original request because if I use axios to the 3rd party api I get a cors error.
  getLogs = () => {
    const { user } = this.props.auth;
    fetch(`http://127.0.0.1:60000/onexagent/api/registerclient?name=${user.id}`)
      .then((res) => res.text())
      .then((result) => {
        const regxml = new XMLParser().parseFromString(result);
        if (regxml.attributes.ResponseCode === "0") {
          axios.post(`/api/avaya/register/${user.id}`, regxml);
        }
        console.log(regxml);
      });

    fetch(`/api/avaya/getid/${user.id}`)
      .then((res) => res.text())
      .then((result) => {
        fetch(
          `http://127.0.0.1:60000/onexagent/api/nextnotification?clientid=${result}`
        )
          .then((res) => res.text())
          .then((result) => {
            const xml = new XMLParser().parseFromString(result);
            axios.post(`/api/avaya/getlogs/${user.id}`, xml);
          });
      });
  };

  timer = (time) => {
    const date = new Date(time);
    return `${date.getHours()}:${date.getMinutes()}:${date.getSeconds()}`;
  };

  componentDidMount() {
    this.getLogs();
    this.interval = setInterval(this.getLogs, 5000);
  }

  componentWillUnmount() {
    clearInterval(this.interval);
  }

Let me share my async await solution for that:

Because Your fetch function is pretty similar till the res.text(), I would create an own function (fetchData ) for that, what is returning a Promise, what is resolving a res.text()
Now you can assign an async to your getLogsfunction
Lets reorganise your code above
Error handling is missing here, but take care about it

:
  fetchData = (url) => {
    return new Promise((res, rej) => {
      fetch(url)
        .then(r => r.text())
        .then(text => {
          res(text)
        })
        .catch(e => rej(e));
    });
  }

  getLogs = async () => {
    const { user } = this.props.auth;
    const regxmlPromise = fetchData(`http://127.0.0.1:60000/onexagent/api/registerclient?name=${user.id}`);
    const secondDataPromise = fetchData(`/api/avaya/getid/${user.id}`);

    const resolves = await Promise.all([regxmlPromise, secondDataPromise]);

    const [firstResultStepOne, secondResultStepOne] = resolves || [];

    const secondResultStepTwo = await fetchData(`http://127.0.0.1:60000/onexagent/api/nextnotification?clientid=${secondResultStepOne}`);

    if (firstResultStepOne) {
      const regxml = new XMLParser().parseFromString(firstResultStepOne);
      if (regxml.attributes.ResponseCode === "0") {
        axios.post(`/api/avaya/register/${user.id}`, regxml);
      }
      console.log(regxml);
    }

    if (secondResultStepTwo) {
      const xml = new XMLParser().parseFromString(secondResultStepTwo);
      axios.post(`/api/avaya/getlogs/${user.id}`, xml);
    }
  };


Filter out strings that are short or that are outside the alphabet of a language I wrote this function to remove from the array those words that are less than n values. More precisely, I updated my old function and added some functionality. Since I am a beginner in this programming language and so I could in my code allow for various illogical errors: Old version of my function:

$words = array("ӯтар","ӯро","ӯт","ғариб","афтода","даст", "ра", "I","Hello", "World"); // Simple array for using

function delValByLessOld($array)
{
    return preg_grep('~\A[^qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM]{2,}\z~u', $array);
}

Simple usage:

echo "
";
   print_r(delValByLessOld($words));
echo "
";

Output:

Array
(
    [0] => ӯтар
    [1] => ӯро
    [2] => ӯт
    [3] => ғариб
    [4] => афтода
    [5] => даст
    [6] => ра
)

My new (updated) function:

function delValByLess(&$array, $less = 2, $lang = 'en')
{

    $search =  array('english','eng', 'russian', 'rus', 'tajik', 'taj', 'tjk');
    $replace = array('en', 'en', 'ru', 'ru', 'tj', 'tj', 'tj');

    if(is_string($lang))
    {
        $lang = mb_strtolower($lang);
        $lang = str_replace($search, $replace, $lang);
    }

    if(is_array($lang))
    {
        foreach ($lang as $lan)
        {
            $lan = mb_strtolower($lan);
            $lang[$lan] = str_replace($search, $replace, $lan);
        }
    }

    $languages =
    [
        'en' => 'qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM',
        'ru' => 'ёйцукенгшщзхъфывапролджэячсмитьбюЁЙЦУКЕНГШЩЗХЪФЫВАПРОЛДЖЭЯЧСМИТЬБЮ',
        'tj' => 'ғӯқҳҷӣҒӮҚҲҶӢ'
    ];

    if (is_array($array) && is_numeric($less))
    {
        if(is_string($lang))
        {
            if (isset($languages[$lang]))
            {
                return preg_grep('~\A[^'.$languages[$lang].']{'.$less.',}\z~u', $array);
            }
        }

        if(is_array($lang))
        {
            foreach ($lang as $lan)
            {
                if (isset($languages[$lan]))
                {
                    $array = preg_grep('~\A[^'.$languages[$lan].']{'.$less.',}\z~u', $array);
                }
            }

            return $array;
        }
    }

    return false;
}

Simple usage:

echo "
";
   print_r(delValByLess($words,'3', ['ENGLISH','TAJIK']));
echo "
";

Output:

Array
(
    [4] => афтода
    [5] => даст
)

It took me a while to decipher what you meant with 'less than n values'. What your function actually does: it filters out all words that are either shorter than the specified length (less) or contain any character from the specified language(s).

The function name is very undescriptive. In a small project that may not be an issue, but in larger projects it quickly becomes impossible to remember what every piece of code does. A good name allows you to quickly understand what a function does. A poor name often requires you to look at how it's implemented, which takes a lot more time. What about removeShortAndForeignWords?
The same goes for parameter and variable names. Good names make code easier to understand: $array -> $words, $less -> $minimumLength, $lang -> $languageCode, $languages -> $languageFilters, and so on.
Why do you try to parse language names? That's very difficult to get right (what if I wanted to use American English, or ру́сский?), so the effect is that your function becomes less reliable, not easier to use. It's much more effective to just document which language codes your function supports (en, ru and tj). In a statically typed language I'd use an enum, but here a set of constants should do.
$lang[$lan] = str_replace($search, $replace, $lan); - Here, because you use $lan as a key, you're adding entries. If the input was array('en', 'ru'), you end up with array('en', 'ru', 'en' => 'en', 'ru' => 'ru'), which results in double work (the filters for each language are executed twice).
if(is_array($lang)) - Use else if here.
qwertyuiopasdfghjklzxcvbnm matches a specific keyboard layout, but abcdefghijklmnopqrstuvwxyz is a much more natural order for latin characters. However, since you're using a regex, you can simply use a-zA-Z.
You can remove a lot of code duplication by making your function recursive. If $lang is an array, then your function can call itself (once for every language code in the array) and then return the final result. Then the rest of the code doesn't need to take multiple language codes into account anymore.
The return value behavior of your function is inconsistent. If you pass an invalid language code, it returns false. However, if you pass an array of invalid language codes, it returns the input words array.
Personally I'd be very careful with references in PHP (&$array). I've seen some surprising edge-cases and as far as I know they often degrade performance. In this case, your function is both returning a filtered array, and it's updating the original array (but only if you pass an array of language codes). So in your last example, not only does the output contain 2 russian words, $words has also been replaced with the results array. That's a very surprising side-effect.

As per your request, here's how I would probably have written it. I don't often work with PHP, and I haven't thoroughly tested this, but it should give you a reasonable impression:

const LANG_EN = 'en';
const LANG_RU = 'ru';
const LANG_TJ = 'tj';

/**
 * Takes an array of words and filters out any words that are too short or that contain any characters from the specified language(s).
 *
 * @param array $words The array of words to filter
 * @param number $minimumLength The minimum allowed word length
 * @param string|array $languageCode The language(s) that must be filtered out. The following languages are supported: LANG_EN, LANG_RU and LANG_TJ.
 *
 * @return array The given words, excluding words that are too short and words that are from the specified language(s).
 */
function removeShortAndForeignWords($words, $minimumLength = 2, $languageCode = LANG_EN) {
    if (is_array($languageCode)) {
        foreach ($languageCode as $language) {
            $words = removeShortAndForeignWords($words, $minimumLength, $language);
        }
        return $words;
    }

    $languageFilters = [
        LANG_EN => '[^a-zA-Z]',
        LANG_RU => '[^ёйцукенгшщзхъфывапролджэячсмитьбюЁЙЦУКЕНГШЩЗХЪФЫВАПРОЛДЖЭЯЧСМИТЬБЮ]',
        LANG_TJ => '[^ғӯқҳҷӣҒӮҚҲҶӢ]',
    ];
    if (!isset($languageFilters[$languageCode])) {
        return false;
    }

    return preg_grep('~\A'.$languageFilters[$languageCode].'{'.$minimumLength.',}\z~u', $words);
}


One function two functionalities or two functions each with one functionality Within our project we've used a lot of boolean setOutputValue(char pinNumber, boolean pinValue) kind of functions.

Within those functions we do the following (for example):

hardware_layer:

#define L9823_HIGH  0
#define L9823_LOW   1
void hardware_L9823_setOutput(U8BIT pin, U8BIT value)
{
  /* some SPI interfacing is handled here */
}

IO_layer:

#define IO_ACTIVE   1
#define IO_INACTIVE 0
void io_outputDriver_setOutput(U8BIT pin, U8BIT value)
{
  if (value == IO_ACTIVE)
  {
    hardware_L9823_setOutput(pinMapping[pin], L9823_HIGH);
  }
  else
  {
    hardware_L9823_setOutput(pinMapping[pin], L9823_LOW);
  }
}

component_layer:

#define COMPONENT_ASIMPLEDEVICE_A_ON    1
#define COMPONENT_ASIMPLEDEVICE_A_OFF   0
void component_aSimpleDeviceA_set(U8BIT value)
{
  if (value == COMPONENT_ASIMPLEDEVICE_A_ON)
  {
    io_outputDriver_setOutput(DEVICE_PIN_A, IO_INACTIVE);
  }
  else
  {
    io_outputDriver_setOutput(DEVICE_PIN_A, IO_ACTIVE);
  }
}

#define COMPONENT_ASIMPLEDEVICE_B_ON    1
#define COMPONENT_ASIMPLEDEVICE_B_OFF   0
void component_aSimpleDeviceB_set(U8BIT value)
{
  if (value == COMPONENT_ASIMPLEDEVICE_B_ON)
  {
    io_outputDriver_setOutput(DEVICE_PIN_B, IO_INACTIVE);
  }
  else
  {
    io_outputDriver_setOutput(DEVICE_PIN_B, IO_ACTIVE);
  }
}

unit_layer:

#define COMPLEXDEVICE_ON    1
#define COMPLEXDEVICE_OFF   0
void component_aComplexDeviceWithMultipleComponents_set(U8BIT value)
{
  if (value == COMPLEXDEVICE_ON)
  {
    component_aSimpleDeviceA_set(COMPONENT_ASIMPLEDEVICE_A_ON);
    component_aSimpleDeviceB_set(COMPONENT_ASIMPLEDEVICE_A_ON);
  }
  else
  {
    component_aSimpleDeviceA_set(COMPONENT_ASIMPLEDEVICE_A_OFF);
    component_aSimpleDeviceB_set(COMPONENT_ASIMPLEDEVICE_A_OFF);
  }
}

This is a rather simple example, but in our case we have a total of 4 layers to seperate hardware/io/component and module implementations. So this has to propagate 4 layers down before it reaches the hardware register.

It does give less code to maintain and less comments to maintain (each function has a header of atleast 9 lines of code.

I've raised the concern of having many if-statements while trying to set a hardware register and proposed to start splitting these functions in to each having their own function like so:

boolean setOutputLow(char pinNumber)
{
    if (pinNumber is within limits of output register mapping)
    {
        regMapping[pinNumber] = OUTPUTLOW;
    }
}

and the same ofc. for the High variant. Which seems to save a lot of if-else logic down the road but does increase code size a tiny bit.

Which would be a prefered way of handling these kind of things? Are there even better ways? Or is this the right road to walk?

This would be better if you could post a real extract of the four layers and the real proposed C replacement code rather than pseudocode.

But you might also consider why software four levels up from the hardware even knows about the names of registers and the polarity of signals at pin level. In other words, the upper level might know that it wants to turn on the "bit-digester" (or some such), but only the driver (ie. lower) layer needs to know which register/bit/level that involves.  That is just my general feeling without knowing any details of your project - of course there might be good reasons for this information to propagate upwards in your application, but it needs to be strong reason to propagate very far...

EDIT after OP updated question

There are always many ways to do these things and my guess is that someone senior set down this arrangement and will be unimpressed by you suggesting it should be changed.

However, that doesn't mean you shouldn't try :-)  I can think of a number of approaches:

What you currently have.  This has the advantage of being simple to understand, but is verbose and involves many conditionals.
What others have suggested, namely passing the 'value' straight through without the conditionals.  This would be more efficient but effectively extends a dependency from the hardware level all the way up through these layers.  The dependency is hidden because the values passed are just assumed to be all the same - 1 on, 0 off.  But it is a dependency all the same and so I don't subscribe to this option.
Use a translation at each level that doesn't involve a conditional.  For example in component_deviceA_set

#define COMPONENT_DEVICE_A_MASK    0x1
void component_deviceA_set(U8BIT value)
{
    static const U8BIT level[] = {IO_INACTIVE, IO_ACTIVE};
    const U8BIT output = level[value & COMPONENT_DEVICE_A_MASK];
    io_outputDriver_setOutput(DEVICE_PIN_A, output);
}

When I try this, I get significantly less code and no conditional as long as the compiler cannot inline the call to io_outputDriver_setOutput.  If the compiler can inline that function, it recreates the conditional (looking in the assembler on x86).

So this approach is probably faster than what you have, and the level[] array need not be repeated for every such function.  But on the downside it involves an extra #define and becomes more complicated if there are cases where there are, say, 3 options instead of 2 (ie. what to do about the invalid 4th option covered by a mask of 0x11).
Use the functional approach you suggest.  This leads to  even simpler code than (3) and in my mind gives the nicest result.  It does lead to a doubling of the number of functions but they are all trivial and take no parameters. The noisy #defines (COMPONENT_DEVICE_B_ON, etc) disappear, which is nice.

So (4) looks like the nicest option to me.

Note that if the compiler can see the guts of all of these functions it might be able to optimise away the conditional in option (1).  But I expect they are in different files, so that will not happen.

Note also that using U8BIT instead of int gains nothing and in fact probably makes the code slightly slower, because the compiler will insert byte-to-long sign extensions when passing them around.   These U8BIT parameters are also dishonest, because the value parameters  don't contain a 'bit', they contain a logical on/off state (or conceivably an on/off/standby etc); yes that in general does come down to a bit at the lower level, but the upper level has no business knowing that.  As far as the upper level is concerned it is just turning something on and whether that involves setting one or a hundred bits is not its concern.

Note also that these U8BIT parameters cause warnings in my compiler ("passing argument 1 of 'io_outputDriver_setOutput' with different width due to prototype") due to my default warning settings


Three Numbers in Java Given three positive numbers A, B, and C. Find the number of positive
  integers less than D that are divisible by either of A, B or C.

  Input format

  The 1st argument given is an Integer A. The 2nd argument given is an
  Integer B. The 3rd argument given is an Integer C. The 4th argument
  given is an Integer D.

  Output Format

  Return an Integer X, the number of positive integers that are
  divisible by either of A, B, or C.

  Constraints

  1 <= A, B, C, D <= 1e9

  For Example

  Input:

  A = 2

  B = 3

  C = 4

  D = 10

  Output:

  6

  Explanation:

  positive numbers less than D that are divisible by either of A, B or C are    2, 3, 4, 6, 8, 9

My approach:

import java.math.BigInteger;

public class Solution {
    public int solve(int A, int B, int C, int D) {
        BigInteger numDivA = BigInteger.valueOf((D - 1)/A);

        BigInteger numDivB = BigInteger.valueOf((D - 1)/B);

        BigInteger numDivC = BigInteger.valueOf((D - 1)/C);

        BigInteger newA = BigInteger.valueOf(A);

        BigInteger newB = BigInteger.valueOf(B);

        BigInteger newC = BigInteger.valueOf(C);

        BigInteger newD = BigInteger.valueOf(D);

        BigInteger limit = new BigInteger("1000000000");

        BigInteger lcmAB = (newA.divide(hcf(newA, newB))).multiply(newB);

        BigInteger lcmAC = (newC.divide(hcf(newA, newC))).multiply(newA);

        BigInteger lcmBC = (newB.divide(hcf(newB, newC))).multiply(newC);

        BigInteger lcmABC = lcmAB.divide(hcf(lcmAB,newC)).multiply(newC);

        BigInteger numDivAB = newD.subtract(BigInteger.ONE).divide(lcmAB);

        BigInteger numDivBC = newD.subtract(BigInteger.ONE).divide(lcmBC);

        BigInteger numDivAC = newD.subtract(BigInteger.ONE).divide(lcmAC);

        BigInteger numDivABC = newD.subtract(BigInteger.ONE).divide(lcmABC);

        BigInteger numTotal = (((((numDivA.add(numDivB)).add(numDivC)).subtract(numDivAB)).subtract(numDivBC)).subtract(numDivAC)).add(numDivABC);

        return (numTotal.mod(limit)).intValue();
    }

    private BigInteger hcf (BigInteger a, BigInteger b) {

        BigInteger hcf = a.gcd(b);

        return hcf;
    }

}

I have the following questions with regards to the above code:

How can I further improve my approach?
Is there a better way to solve this question?
Are there any grave code violations that I have committed?
Can space and time complexity be further improved?
Have I gone too overboard by using BigInteger?

Reference

1e9 is only 1 billion, which will fit into a standard integer range of 2 billion some.

The hcf function is unexpected, because it's just a wrapper for BigInteger. I think just using the built in GCD function would be better than making your own method that calls a standard function. If it's critical to the algorithm understanding that we think of a highest common factor instead of a greatest common denominator, then keep the function. If you do keep it, though, use the full name instead of an abbreviation i.e. private BigInteger highestCommonFactor.

I looked at your code and expected to see a loop. I'm assuming you've worked out the math to do this without loops, which is good and probably more efficient. But, I don't really know how this would work, and would be worried someone else looking at the code would have the same problem. A code comment indicating what mathematical principles, or even the name of a theorem, would help immensely.


Finding surrounding tiles for all tiles What can I do to improve performance of this function? So far I've changed stuff as local as possible and made a break if all possible surrounding tiles have been found. Also I use a separate variable to track the length of a list instead of len().

The usage of this is to find surrounding tiles for all tiles.

def getsurroundings(tiles):
    for t in tiles:
        currentrect = Rect(t.rect[0] - t.rect[2] / 2, t.rect[1] - t.rect[3] / 2, t.rect[2] * 2, t.rect[3] * 2)
        check = currentrect.colliderect
        surroundings = t.surroundings
        append = t.surroundings.append
        surroundingscount = t.surroundingscount
        for t2 in tiles:
            if check(t2.rect) and not t == t2 and not t2 in surroundings:
                append(t2)
                t2.surroundings.append(t)
                surroundingscount+=1
                t2.surroundingscount+=1
            if surroundingscount == 8:
                break
    return tiles

Here's the code without localisations which might be easier to read:

def getsurroundings(tiles):
    for t in tiles:
        currentrect = Rect(t.rect[0] - t.rect[2] / 2, t.rect[1] - t.rect[3] / 2, t.rect[2] * 2, t.rect[3] * 2)
        for t2 in tiles:
            if currentrect.colliderect(t2.rect) and not t == t2 and not t2 in t.surroundings:
                t.surroundings.append(t2)
                t2.surroundings.append(t)
                t.surroundingscount+=1
                t2.surroundingscount+=1
            if t.surroundingscount == 8:
                break
    return tiles

The problem here isn't in the details of the code, but in the algorithm. For every tile, you check all tiles, so for n tiles, your code has to perform n^2 checks, which is a lot. The only way to significantly reduce the running time of your code, is to make the search smarter. If you can make your use case a bit more visual, we might be able to come up with something.


Detecting the mime type using an agnostic method This function is used to get the mime type of any given file. The intended purpose was to have it working within any of our servers that vary from the latest PHP version down to PHP 4.

/**
 * Get the mime type for any given file.
 *
 * @param str $filename Path to the file including the filename
 *
 * @return mixed bol|arr Either false or the array provided by the PHP function
 */
function getMimeType($filename) {
  $mimetype = false;

  if (function_exists('finfo_fopen')) {
    $mimetype = finfo_fopen($filename);
  } elseif (function_exists('getimagesize')) {
    $mimetype = getimagesize($filename);
  } elseif (function_exists('exif_imagetype')) {
    $mimetype = exif_imagetype($filename);
  } elseif (function_exists('mime_content_type')) {
    $mimetype = mime_content_type($filename);
  }

  return $mimetype;
}

While the function is already tested on different PHP versions, I'm trying to ascertain if the methodology implemented can be improved, thus leading to code reduction and preventing any redundant verifications.

Can this function receive any type of improvement?

General Issues

If you are going to call something getMimeType then it should return exactly that.  This is probably best defined by Multipurpose Internet Mail Extensions RFC 2046.  Read the wikipedia page for more general information and related RFCs.

As Corbin commented on this implementation is broken due to the number of assumptions that are required to consume the information provided by this function.  You really should return just a string with the mime type or an array with the primary type and subtype.  Whatever you return should be consistent with your documentation.

You could consider some of the following instead of returning false:

Throw an exception.
Trigger an error and default the MIME type.
Create a second function hasMimeType to avoid exception/errors.

Specific Issues

finfo_fopen

I think finfo_fopen should probably be finfo_open.

Its usage is something like:

$finfo = finfo_open(FILEINFO_MIME_TYPE);
return finfo_file($finfo, $filename);

However there are some catches.  FILEINFO_MIME_TYPE has only been defined since PHP 5.3.0.  You can use version_compare to determine which version of php you are using (If the version_compare function doesn't exist you are using PHP < 4.1.0).

if (version_compare(PHP_VERSION, '5.3.0') >= 0) { /* Do Stuff */ }

getimagesize

Only the mime index from the returned array should be used.

exif_imagetype

The string returned from this does not conform to the MIME type format.  You will have to massage this data to return something appropriate.


PHP spell checker with suggestions for misspelled words I built a simple PHP spellchecker and suggestions app that uses PHP's similar_text() and levenshtein() functions to compare words from a dictionary that is loaded into an array.

How it works is first I load the contents of the dictionary into an
array.
I split the user's input into words and spell check each of the
words.
I spell check by checking if the word is in the array that is the
dictionary.
If it is, then I echo a congratulations message and move on.
If not, I iterate through the dictionary-array comparing each word, in the dictionary-array, with the assumed misspelling.
If the inputted word, in lower-case and without punctuation, is 90%
or more similar to a word in the dictionary array, then I copy that
word from the dictionary array into an array of suggestions.
If no suggestions were found using the 90% or higher similarity
comparison, then I use levenshtein() to do a more liberal comparison
and add suggestions to the suggestions array.
Then I iterate through the suggestions array and echo each
suggestion.

I noticed that this is running slowly. Slow enough to notice. And I was wondering how I could improve the speed and efficiency of this spell checker.

Any and all changes, improvements, suggestions, and code are welcome and appreciated.

Here is the code (For syntax highlighted code, please visit here):

<?php
function addTo($line){
    return strtolower(trim($line));
}
$words = array_map('addTo', file('dictionary.txt'));
$words = array_unique($words);
function checkSpelling($input, $words){
$suggestions = array();
if(in_array($input, $words)){
    echo "you spelled the word right!";
}
else{
    foreach($words as $word){
        $percentageSimilarity=0.0;
        $input = preg_replace('/[^a-z0-9 ]+/i', '', $input);
        similar_text(strtolower(trim($input)), strtolower(trim($word)), $percentageSimilarity);
        if($percentageSimilarity>=90 && $percentageSimilarity<100){
                if(!in_array($suggestions)){
                    array_push($suggestions, $word);
        }
        }
    }
if(empty($suggestions)){
    foreach($words as $word){
    $input = preg_replace('/[^a-z0-9 ]+/i', '', $input);
    $levenshtein = levenshtein(strtolower(trim($input)), strtolower(trim($word)));
    if($levenshtein<=2 && $levenshtein>0){
         if(!in_array($suggestions)){
           array_push($suggestions, $word);
          }
        }
    }
}
    echo "Looks like you spelled that wrong. Here are some suggestions:
";
         foreach($suggestions as $suggestion){
             echo "
".$suggestion."
";
         }
    }
}
if(isset($_GET['check'])){
    $input = trim($_GET['check']);
    $sentence='';
    if(stripos($input, ' ')!==false){
    $sentence = explode(' ', $input);
    foreach($sentence as $item){
        checkSpelling($item, $words);
    }
    }
    else{
         checkSpelling($input, $words);
    }
 }
 ?>
<!Doctype HTMl>

        <meta charset="utf-8" />
        <title>Spell Check</title>

        <form method="get">
             <input type="text" name="check" autocomplete="off" autofocus />
        </form>

Here are a couple of tweaks that could help performance:

Rather than storing the dictionary in-memory, offload that to a database (potentially even caching commonly misspelled words as an optimization)
Ignore words under a certain length
(for example, MySQL's fulltext searching ignores words with fewer than 4 characters by default)

The thing that concerns me most with your algorithm is how much time it would take to compare every single word in the dictionary.  This problem compounds with more words in the search query.

There has to be a way to quickly filter the dictionary to a smaller list of higher probability similarities (i.e. by word length, first letter, etc. ?)


Error handling for a simple 'fetch some data, then save the data' Node function This is a fairly simple bit of code:

async function fetchData() {
    console.log("fetching data");
    let json;
    try {
        const data = await fetch(endPoint);
        json = await data.json();
    }
    catch (error) {
        console.error(error);
        handleError(error);
    }

    try {
        await saveData(json);
    } catch (error) {
        console.error(error);
        handleError(error);
    }
}

console.info("start running");
setInterval(fetchData, config.newsapi.polling_interval);
console.info("application is running");

What we're doing is - every 15 minutes or so, we're polling an API endpoint, and then saving the results into a database.

So there's two main kinds of errors we might get here:

The API I fetched the data from might return some kind of error - it might be down, or my API key might be wrong, or I've gone over the rate limit or whatever.
The database might return some kind of error. For all the same reasons, but a completely different service.

Now in terms of handling these errors, for now all I'll be doing is logging the error, and send me an email.

But I can imagine, that in the future, I might want to handle these two distinct kinds of errors differently. As such, I've created two different try catch blocks, that gives me the flexibility to handle them differently.

But it does look a bit messier, in my opinion. The other way I could write this is:

async function fetchData() {
    console.log("fetching data");
    try {
        const data = await fetch(endPoint);
        const json = await data.json();
        await saveData(json);
    }
    catch (error) {
        console.error(error);
        handleError(error);
    }

}

console.info("start running");
setInterval(fetchData, config.newsapi.polling_interval);
console.info("application is running");

This looks so much tidier. But in which case - it seems like the error handling is a bit trickier.

I guess I could put a switch statement in the catch block handle different errors differently, but is that a bit messy? Any suggestions or guidelines for navigating this?

In my opinion it is much better to place the saveData call inside the first try-catch block.

This avoids calling saveData with an undefined value in case an exception occurs in calls to fetch or data.json, and gives the json variable a clean scope.

Depending on the granularity needed for error handling I would prefer to surround

await saveData(json);

with try-catch if necessary. However, a switch statement could also be a natural approach if you have a clear distinction on the exceptions thrown (e.g. error numbers).


C++ inorder traversal of binary tree The challenge was to create a function which performs an in order traversal without using recursion. What I came up with was to use a stack and some clever popping and pushing. I was wondering if there was any way to improve the following solution. Specifically a better way to implement my if(s.empty()) break; line.

vector<int> inorderTraversal(TreeNode* root) {
        stack<TreeNode*> s;
        vector<int> ans;
        if(root == nullptr) return ans;
        s.push(root);
        while(!s.empty()){
            TreeNode* x = s.top();
            s.pop();
            if(x != nullptr && (x -> right != nullptr || x -> left != nullptr)){
                s.push(x -> right);
                s.push(x);
                s.push(x -> left);
            }else{ //leaf node
                if(x != nullptr)
                    ans.push_back(x -> val);

                if(s.empty()) break;

                if(s.top() != nullptr)
                    ans.push_back(s.top() -> val);
                s.pop();
            }
        }
        return ans;
    }

Your algorithm does seem to work, but it is very complex.  You are pushing nullptr onto the stack when a node has only one child, so you have to check for x != nullptr (which you do),   and if the top of the stack is nullptr (which again, you do).

A better approach would be:

start with an empty vector
start with an empty stack
start with root as the current node
while you are at a valid node, or if the stack is not empty:

if you are at a valid node:

if it has a left child,

push the current node onto the stack
move to the left child.

otherwise (it doesn't have a left child)

append its value to your vector
move to the right child (even if it doesn't exist)

otherwise (you have moved to a nullptr):

pop a node from the stack
append its value to your vector
move to the right child (even if it doesn't exist)

Notice you never push a nullptr onto the stack.  You never need to push a node onto the stack unless you need to return to it.  When you do return to it, you immediately process it by appending its value and moving to its right child.

Also notice that an empty tree is no longer a special case.


"Multiplying" 2 lists of Strings together in Java 8 The following method is part of a class that imports zip archives, and part of that class's job is to verify that a given zip archive contains "complete filesets". A "complete fileset" is one such that, for a given file name prefix, there exists a file in the zip archive with that prefix and each suffix. There are always 3 suffixes and they are always the same 3.

For example, say there are file names prefixes A, B, and C, and the suffixes are X, Y, and Z. Then the file names found in the zip archive should be AX, AY, AZ, BX, BY, BZ, CX, CY, and CZ.

This method is used to create the list of expected file names in order to verify that they all exist within the zip archive.

Here is the method in question:

public static List<String> multiply( List<String> list1, List<String> list2 ) {
    return list1.stream()
        .map( s1 -> {
            return list2.stream()
                .map( s2 -> s1 + s2 )
                .collect( Collectors.toList() );
        })
        .flatMap( List::stream )
        .collect( Collectors.toList() );
}

I was wondering if, maybe, there is a clearer or more concise way to accomplish this? I realize it can be done with a couple of for loops, but I would prefer using Java 8 streams, as I would like to practice and learn the API. Also, I'm not sure if "multiply" is the right thing to call this operation.

Here is the unit test for the method if it helps to clarify the usage and intent:

@Test
public void testMultiply() {
    List<String> list1 = Lists.newArrayList( "A", "B", "C" );
    List<String> list2 = Lists.newArrayList( "D", "E", "F" );
    List<String> result = Lists.newArrayList(
        "AD", "AE", "AF",
        "BD", "BE", "BF",
        "CD", "CE", "CF");
    Assert.assertEquals( result, multiply( list1, list2 ) );
}

Method name

I agree with you, multiply() does sound a bit odd for lists of non-numbers... perhaps combine() or cartesianProduct() are arguably better names?

Lambdas

'Single-shot' lambda definitions does not require the use of { return ...; }:

.map( s1 -> list2.stream().map( s2 -> s1 + s2 ).collect( Collectors.toList() ) )

Actually, when you are mapping an element to a Collection, only to flatMap() it immediately afterwards, you can probably do without the inner collect():

return list1.stream()
            .flatMap( s1 -> list2.stream().map( s2 -> s1 + s2 ) )
            .collect( Collectors.toList() );


Code Review of Haskell PBKDF2 Moved from Programmers.SE.

I have written a new version of the PBKDF2 algorithm in Haskell. It passes all of the HMAC-SHA-1 test vectors listed in RFC 6070, but it is not very efficient. How can I improve the code? I plan to upload it to Github (and maybe Hackage) once it is improved.

{-# LANGUAGE BangPatterns #-}
{- Copyright 2013, G. Ralph Kuntz, MD. All rights reserved. LGPL License. -}

module Crypto where

import Codec.Utils (Octet)
import qualified Data.Binary as B (encode)
import Data.Bits (xor)
import qualified Data.ByteString.Lazy.Char8 as C (pack)
import qualified Data.ByteString.Lazy as L (unpack)
import Data.List (foldl')
import Data.HMAC (hmac_sha1)
import Text.Bytedump (dumpRaw)

-- Calculate the PBKDF2 as a hexadecimal string
pbkdf2
  :: ([Octet] -> [Octet] -> [Octet])  -- pseudo random function (HMAC)
  -> Int  -- hash length in bytes
  -> String  -- password
  -> String  -- salt
  -> Int  -- iterations
  -> Int  -- derived key length in bytes
  -> String
pbkdf2 prf hashLength password salt iterations keyLength =
  let
    passwordOctets = stringToOctets password
    saltOctets = stringToOctets salt
    totalBlocks =
      ceiling $ (fromIntegral keyLength :: Double) / fromIntegral hashLength
    blockIterator message acc =
      foldl' (\(a, m) _ ->
        let !m' = prf passwordOctets m
        in (zipWith xor a m', m')) (acc, message) [1..iterations]
  in
    dumpRaw $ take keyLength $ foldl' (\acc block ->
      acc ++ fst (blockIterator (saltOctets ++ intToOctets block)
                      (replicate hashLength 0))) [] [1..totalBlocks]
  where
    intToOctets :: Int -> [Octet]
    intToOctets i =
      let a = L.unpack . B.encode $ i
      in drop (length a - 4) a

    stringToOctets :: String -> [Octet]
    stringToOctets = L.unpack . C.pack

-- Calculate the PBKDF2 as a hexadecimal string using HMAC and SHA-1
pbkdf2HmacSha1
  :: String  -- password
  -> String  -- salt
  -> Int  -- iterations
  -> Int  -- derived key length in bytes
  -> String
pbkdf2HmacSha1 =
  pbkdf2 hmac_sha1 20

The comments on SO give some good hints. The most critical part is obviously blockIterator, so let's make it a separate function:

blockIterator
    :: ([Octet] -> [Octet] -> [Octet])  -- ^ pseudo random function (HMAC)
    -> [Octet]                          -- ^ password octets
    -> Int                              -- ^ iterations
    -> [Octet]                          -- ^ m
    -> [Octet]                          -- ^ a
    -> [Octet]

The implementation using folds on a list is nice and concise, but processing the list adds some unnecessary overhead, and prevents possible optimizations. If we instead rewrite it as a recursive function

blockIterator prf passwordOctets = loop
  where
    loop i m a | i == 0     = a
               | otherwise  = let m' = prf passwordOctets m
                                  a' = zipWith xor a m'
                               in a' `deepseq`
                                  m' `deepseq`
                                  loop (i - 1) m' a'

it gets somewhat faster. I also added deepseq on the octet lists so that they're fully evaluated at each round. Introducing the recursion inside loop, instead of making the whole blockIterator recursive allows GHC to inline it.

Measuring performance can be done using the criterion package:

import Criterion.Main

-- ...

main = do
    let stdtest n = pbkdf2HmacSha1 "password" "salt" n 20
    defaultMain [ bgroup "stdtest" $
                    map (\n -> bench (show n) (nf stdtest n))
                        [1, 2, 4096, 65536 ]
                ]

However, the far most time-consuming part is still processing lists inside blockIterator. Using unboxed ST arrays would make the code way faster. There would be no memory allocation in the loop, no need to force evaluation. The problem is, hmac_sha1 is implemented using list, so we'd need another optimized, ST-based implementation.


Minimize Hard Coding in React Native Can anybody help me to minimize my hard coding? Also please give me any suggestions for improving my code for the future?

My Source Code:

  export default class App extends React.Component {
  render() {
    return (
      <View style={styles.container}>
          <View style={styles.border}>
          <Image source={{uri:'https://images.unsplash.com/photo-1579551356536-e2d17fe1c7fa?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=400&q=80'}} style={styles.image}/>
          </View>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#fff',
    padding: 8,
  },
  border:{
    width: 200,
    height: 300,
    borderTopEndRadius: 50,
    borderBottomStartRadius: 50,
    borderRadius: 15,
    backgroundColor: 'red',
    shadowColor: '#000',
    shadowOffset: { width: 0, height: 2 },
    shadowOpacity: 0.8,
    shadowRadius: 8,
  },
  image:{
    width:'100%',
    height: '100%',
    borderTopEndRadius: 50,
    borderBottomStartRadius: 50,
    borderRadius: 15,
  }
});

To clean the main render, we can extract the below JSX into a method and render that method into the main render.

<View style={styles.container}>
          <View style={styles.border}>
          <Image source={{uri: URI}} style={styles.image}/>
          </View>
      </View>)

For image url, instead of assigning it directly into the <Img > tag, we can declare a static property or even in the constructor or state, then can use accordingly.

Further refactoring can be done by extracting the StyleSheet into a separate Style.js.

All the colors used can be used from a single file.

Modified Code:

  export default class App extends React.Component {
  static URI = 'https://images.unsplash.com/photo-1579551356536-e2d17fe1c7fa?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=400&q=80'

  defaultView = () => {
   return(
     <View style={styles.container}>
          <View style={styles.border}>
          <Image source={{uri: URI}} style={styles.image}/>
          </View>
      </View>)}

  render() {
    return (
      {this.defaultView()}
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#fff',
    padding: 8,
  },
  border:{
    width: 200,
    height: 300,
    borderTopEndRadius: 50,
    borderBottomStartRadius: 50,
    borderRadius: 15,
    backgroundColor: 'red',
    shadowColor: '#000',
    shadowOffset: { width: 0, height: 2 },
    shadowOpacity: 0.8,
    shadowRadius: 8,
  },
  image:{
    width:'100%',
    height: '100%',
    borderTopEndRadius: 50,
    borderBottomStartRadius: 50,
    borderRadius: 15,
  }
});


HackerEarth Challenge Find Maximum Taste of Fruits Problem Statement:

  Samu had got N fruits. Sweetness of ith fruit is given by A[i]. Now
  she wants to eat all the fruits , in such a way that total taste is
  maximised.

  Total Taste is calculated as follow (Assume fruits sweetness array
  uses 1-based indexing) :

int taste=0;
for(int i=2;i<=N;i++){
    if (A[i]-A[i-1] >= 0){
       taste = taste + i*(A[i]-A[i-1]);
    }
    else{
       taste = taste + i*(A[i-1]-A[i]);
    }
}

  So, obviously the order in which she eat the fruits changes the Total
  taste. She can eat the fruits in any order. Also once she start eating
  a fruit she will finish that one before moving to some other fruit.

  Now she got confused in deciding the order in which she should eat
  them so that her Total taste is maximized. So she ask for help from
  you. Help her find the maximum taste she can have , if she can eat the
  fruits in any order.

  Input Format:

  First line contain number of test cases T. First line of each test
  case contains N, denoting the number of fruits. Next line contain N
  space separated integers denoting the sweetness of each fruit.

  Output Format:

  For each test case you need to print the maximum taste Samu can have
  by eating the fruits in any order she want to.

  Constraints:

  \$1 \le T \le 10\$   \$1 \le N \le 15\$

  Sweetness of each fruit, A[i] lies between 1 to 100 for 1 ≤ i ≤ N

  Sample Input:

1
3
2 5 4

  Sample Output:

13

  Explanation:

[2,4,5] -> 2*(2) + 3*(1) = 7
[2,5,4] -> 2*(3) + 3*(1) = 9
[5,4,2] -> 2*(1) + 3*(2) = 8
[5,2,4] -> 2*(3) + 3*(2) = 12
[4,5,2] -> 2*(1) + 3*(3) = 11
[4,2,5] -> 2*(2) + 3*(3) = 13

  The maximum is 13 that can be obtained by eating the fruits according to
  sweetness order [4,2,5].

from itertools import permutations
for _ in xrange(int(raw_input())):
    n=int(raw_input())
    val=map(int,raw_input().split())
    max_ = 0
    for A in permutations(val):
        taste=0;
        for i in xrange(1,len(A)):
            taste = taste + (i+1)*abs(A[i]-A[i-1])
        max_= max(max_,taste)
    print max_

You are searching over the full set of permutations, which leads to a time complexity of \$O(n!)\$, which is rarely a good thing. This seems like the type of problem that would benefit from a dynamic programming approach. I don't find it entirely satisfactory, as it uses a top-down approach, but it is much more efficient than your implementation, so here it goes:

The basic idea is that, if you take a subset \$A\$ of \$m\$ items, and compute the \$m\$ values \$t_{A,j}\$, being the "best taste using the items in \$A\$ and ending in the \$j\$-th item, you can compute \$t_{A^*, m+1}\$, where \$A^*\$ is the set \$A\$ augmented with an extra item not in that set, easily. This is kind of confusing, I know, but below I am indexing memoized solutions by (items_remaining_to_be_used, last_item_used), and building the total taste by adding one of the remaining items at a time. It may be easier to look at the code:

def top_down(seq):
    memo = {}
    for idx, item in enumerate(seq):
        key = (frozenset(seq[:idx] + seq[idx+1:]), item)
        memo[key] = 0
    mult = 1
    best_taste = None
    while best_taste is None:
        mult += 1
        new_memo = {}
        for (items, last), taste in memo.items():
            items = list(items)
            for idx, item in enumerate(items):
                new_taste = taste + mult * abs(item - last)
                new_set = frozenset(items[:idx] + items[idx+1:])
                if not new_set:
                    best_taste = max(new_taste,
                                     0 if best_taste is None else best_taste)
                else:
                    new_key = new_set, item
                    new_memo[new_key] = max(new_memo.get(new_key, 0),
                                            new_taste)
        memo = new_memo
    return best_taste

Comparing to a function implementing your naive approach:

def naive(seq):
    best_taste = 0
    for perm in permutations(seq):
        taste = 0
        for idx, (this, that) in enumerate(zip(perm[:-1], perm[1:])):
            taste += (idx + 2) * abs(this - that)
        best_taste = max(best_taste, taste)
    return best_taste

The timing differences start out small:

a = [random.randint(100) for _ in range(5)]
%timeit naive(a)
1000 loops, best of 3: 215 µs per loop
%timeit top_down(a)
1000 loops, best of 3: 262 µs per loop

But things soon favor the top_down implementation massively:

a = [random.randint(100) for _ in range(10)]
%timeit naive(a)
1 loops, best of 3: 10.4 s per loop
%timeit top_down(a)
10 loops, best of 3: 44.6 ms per loop

That's over a 200x improvement! And of course:

>>> naive(a) == top_down(a)
True

And even the largest problem in your challenge can be solved in a reasonable amount of time, not sure if it will be fast enough though:

a = [random.randint(100) for _ in range(15)]
%timeit top_down(a)
1 loops, best of 3: 3.76 s per loop


How can this simple coroutine implementation be improved? This is a quick and dirty implementation of coroutines that implements yield by saving and restoring the stack to and from the heap. Here's an earlier version, which does the most naive possible thing, and just allocates enough space on the stack for every possible coroutine. The relevant bits from the current code are below, and the whole thing is here.

I'm open to feedback both on pretty much anything: completely different approaches, how to restructure this approach, bugs, idiomatic C style pointers, etc.

An obvious performance improvement would be to use ucontext, but that seems to be deprecated and somewhat broken on Mac OS X (although it works fine on the linux distros I've tried). I'm not sure what the next step is to make this 'better'.

typedef void (*coroutine_cb)();

static void scheduler(coroutine_cb);
static int spawn(coroutine_cb);
static void yield(int);

jmp_buf scheduler_jmp;
void *scheduler_rbp;
int coro_pid;

coroutine_cb scheduler_next_coro;

#define MAX_COROS 100

struct {
  jmp_buf jmp;
  void *stack;
  long stack_sz;
} coroutines[MAX_COROS];

static void scheduler(coroutine_cb coro)
{
  printf("starting the scheduler...\n");
  static int max_pid = 1;

  // we move down 0x1000 to give scheduler space to call functions and allocate stack variables without having them get
  // overwritten by the memcpy below. Before we did this, we had to manually copy memory instead of using memcpy
  // because we would overwrite memcpy's stack
  scheduler_rbp = __builtin_frame_address(0) - 0x1000;
  scheduler_next_coro = coro;
  int value = setjmp(scheduler_jmp);
  // value == 0 means just starting
  // value == -1 means spawning new coroutine
  // value positive means hop back to a specific pid
  if (value == 0 || value == -1)
  {
    coro_pid = max_pid++;
    printf("about to run coro %d...\n", coro_pid);
    char *buf = alloca(0x2000); // was 0x1000 when we didn't allocate extra space for scheduler stack
    asm volatile("" :: "m" (buf));
    scheduler_next_coro();
    assert(0);
  }
  else
  {
    printf("jumped back to scheduler (pid --> %d) (coro_pid --> %d)...\n", value, coro_pid);
    // restore coroutine marked by value (pid)
    coro_pid = value;

    int stack_sz;
    stack_sz = coroutines[coro_pid].stack_sz;
    memcpy(scheduler_rbp - stack_sz, coroutines[coro_pid].stack, stack_sz);
    longjmp(coroutines[coro_pid].jmp, 1);
    assert(0);
  }
}

static void yield(int pid)
{
  // take current rbp
  void *rbp = __builtin_frame_address(0);
  void *fudgy_rsp = (char *)rbp - 0x100;
  assert(scheduler_rbp > rbp);

  long stack_sz = (char *)scheduler_rbp - (char *)fudgy_rsp;
  void *stack = malloc(stack_sz);

  /*
   * Marek: check how overflowing stack actually works, because
   * we're actually copying data beyond our stack frame.
   */
  memcpy(stack, fudgy_rsp, stack_sz);
  coroutines[coro_pid].stack = stack;
  coroutines[coro_pid].stack_sz = stack_sz;

  if (!setjmp(coroutines[coro_pid].jmp))
  {
    longjmp(scheduler_jmp, pid);
    assert(0);
  }
  else
  {
    // our stack is already good to go at this point
    return;
  }
}

/*
 *
 */
static int spawn(coroutine_cb coro)
{
  scheduler_next_coro = coro;
  yield(-1);
  return 0; // need to get pid
}

int main()
{
  scheduler(f);
  assert(0);
  return 0;
}

Things you did well on:

Overall, this looks like a pretty decent little bit of code.  It looks like some research went into this
I like the comments.  Everywhere I was confused on something, there was a comment to clarify.  You also didn't go overboard with them!

Things you could improve on:

Preprocessor:

Group all of your #defines at the top of your code, just after your #imports.  Otherwise you will have to jump around your code to find them.

Initialization:

typedef your structs.

struct {
  jmp_buf jmp;
  void *stack;
  long stack_sz;
} coroutines[MAX_COROS];

The typedef means you no longer have to write struct all over the place. That not only saves keystrokes, it also can make the code cleaner since it provides a smidgen more abstraction.
You can initialize some variables right away.

int stack_sz;
stack_sz = coroutines[coro_pid].stack_sz;

int stack_sz = coroutines[coro_pid].stack_sz;

Syntax:

You use print() where it is unneeded.

printf("starting the scheduler...\n");

You can use puts() instead.

puts("Starting the scheduler...");

Memory:

You allocated memory to stack, but never free it.

void *stack = malloc(stack_sz);

Failure to deallocate memory using free leads to buildup of non-reusable memory, which is no longer used by the program. This wastes memory resources and can lead to allocation failures when these resources are exhausted.

free(stack);

Comments:

You could use the /* ... */ comment format instead of the // ... format when commenting over multiple lines.

// value == 0 means just starting
// value == -1 means spawning new coroutine
// value positive means hop back to a specific pid

You could also condense your comments down a bit.

/*
 * If 'value' is 0, it is just starting; if -1 it is spawning.
 * If 'value' is positive, we "hop" to a specific PID.
 */

Resources:

Coroutine implementations for C
Coroutines in C
Building Coroutines
Libtask: a Coroutine Library for C and Unix


Binary Search Tree implementation using smart pointers I have implemented below code for binary search tree implementation using shared pointer. At present, I have considered only integers. It supports insertion and deletion of values. Also, a print method is implemented for inorder traversal.

Please review and provide your suggestions.

class Bst {
    struct node {
        int val;
        shared_ptr<node> left, right;
        node(int v) : val(v), left(nullptr), right(nullptr){}
    };
    shared_ptr<node> root;
    shared_ptr<node> _insert(shared_ptr<node> curr, int val);
    shared_ptr<node> _del(shared_ptr<node> curr, int val);
    shared_ptr<node> findmin(shared_ptr<node> curr);
    void _print(shared_ptr<node> curr);

public:
    void insert(int val) { root = _insert(root, val); }
    void del(int val) { root = _del(root, val); }
    void print() { _print(root); cout << '\n'; }
};

shared_ptr<Bst::node> Bst::_insert(shared_ptr<node> curr, int val)
{
    if (curr == nullptr)
        return make_shared<node>(val);

    if (val < curr->val)
        curr->left = _insert(curr->left, val);
    else
        curr->right = _insert(curr->right, val);

    return curr;
}
shared_ptr<Bst::node> Bst::findmin(shared_ptr<node> curr)
{
    while (curr->left)
        curr = curr->left;
    return curr;
}
shared_ptr<Bst::node> Bst::_del(shared_ptr<node> curr, int val)
{
    if (val < curr->val)
        curr->left = _del(curr->left, val);
    else if (val > curr->val)
        curr->right = _del(curr->right, val);
    else {
        if (curr->right == nullptr)
            return curr->left;
        if (curr->left == nullptr)
            return curr->right;
        shared_ptr<node> temp = findmin(curr->right);
        curr->val = temp->val;
        curr->right = _del(curr->right, curr->val);
    }
    return curr;
}
void Bst::_print(shared_ptr<node> curr)
{
    if (curr == nullptr)
        return;
    _print(curr->left);
    cout << curr->val << " ";
    _print(curr->right);
}

please do not using namespace std;
why has Bst::findmin no prefix _?
why do you use std::shared_ptr<> at all? std::shared_ptr is ment to represent shared ownership. This makes no sense for tree nodes, you're not going to have two trees with the same nodes. std::unique_ptr<> is a better candidate.
it is generally good to separate the data structure from its serialization. Make print a separate function and also handle the member printing outside of the class.

Edit
As you had trouble getting the unique_ptr running, here's what works for me.

Modify the node to use unique_ptr:

struct node
{
    int val;
    std::unique_ptr<node> left;
    std::unique_ptr<node> right;

    // c'tor left out
};

Also, use a unique_ptr for root. You'll receive heavy compiler complaints,
because a unique_ptr cannot be simply copied (one of the instances must go invalid, as it's unique). I suggest altering the private functions:

void _insert(std::unique_ptr<node>& curr, int val);
void _del(std::unique_ptr<node>& curr, int val);

Sample implementation for _insert:

void Bst::_insert(std::unique_ptr<node>& curr, int val)
{
    if (!curr)
    {
       curr = std::make_unique<node>(val);
       return;
    }

    if (val < curr->val)
    {
        _insert(curr->left, val);
    }
    else
    {
        _insert(curr->right, val);
    }
}

Your public insert method would be something like:

void insert(int val)
{
    _insert(root, val);
}

Also: add a find function to see if an element is in the tree. Use the find function to write some simple tests. You may rename del to erase to be closer at the standard naming.

Edit 2 Separate output from data storage:

A friend function is relatively inflexible, as you cannot easily exchange the printing algorithm. You may implement iterators (hard) or provide a traverse function. I'd go for the second approach, as you already have the algorithm in _print: rename print to traverse and pass it a function pointer, then replace the output to std::cout by a call to the function pointer.

class Bst
{
...
    // traverse with function pointer
    void traverse(void (*func)(int));
};

int main()
{
    Bst b;
    // insert data ...

    b.traverse([](auto v){ std::cout << v << " "; });
    std::cout << "\n";
}


Sending an email via Outlook I have a function which sends an email via outlook when given text, a subject, and recipients shown below:

def __Emailer(text, subject, recipient, auto=True):
    import win32com.client as win32

    outlook = win32.Dispatch('outlook.application')
    mail = outlook.CreateItem(0)
    if type(recipient) == type([]):
        for name in recipient:
            mail.Recipients.Add(name)
    else:
        mail.To = recipients
    mail.Subject = subject
    mail.HtmlBody = text
    if auto:
        mail.send
    else:
        mail.Display(True)

Now, I know that type comparisons in Python are strongly discouraged, but I can't think of a better way to do this.  I want it to send to multiple addresses if recipient is a list using the mail.Recipients.Add(name), and mail.To if there is only one name (not given as a list).  How can I accomplish this without using nasty type comparisons?

In Python, it is part of the mentality to check for behavior rather than for exact type.

Your method doesn't even need recipients to be a list—it can be any iterable (e.g. a set) so it can be used in the for loop. Strings, however, are also iterable, so you need to tell them apart.

One way to check if something is a string is to look for a string-specific method, such as strip.

def __Emailer(text, subject, recipients, auto=True):
    import win32com.client as win32

    outlook = win32.Dispatch('outlook.application')
    mail = outlook.CreateItem(0)

    if hasattr(recipients, 'strip'):
        recipients = [recipients]

    for recipient in recipients:
        mail.Recipients.Add(recipient)

    mail.Subject = subject
    mail.HtmlBody = text

    if auto:
        mail.send
    else:
        mail.Display(True)


Bulls and Cows C++ beginner project I'm not a beginner programmer as I have experience in Node and Ruby, but I'm a beginner when it comes to C++. And as C++ is quite different from these two, I figured I could use any tips, tricks, hints and pointers (hah!) to learn how to actually write C++ and to get the most out of the language. I feel a bit like I'm trying to write JS but in C++ syntax.

To get me started I've written a Bulls and cows game which works as I intended it to, but like I said above, I need help in improving the code generally as well as actually utilizing the built in C++ methods.

I'm wondering if it's worthwhile to write a separate function for say the random number generation, or would that be a waste of memory?

I've also seen that many in the C++ community seem to favour comments instead of descriptive variable/function names.
Example:

int a = 3.14159265359; // pi with 11 decimal numbers

instead of:

int pi_eleven_decimals = 3.14159265359;

Is that merely a question of taste or is it simply how it's done in C++?

The bulls and cows game code:

#include "pch.h"
#include <iostream>
#include <time.h>
#include <string>
#include <sstream>
#include <vector>

using namespace std;

int main() {
    int bull;
    int cow;
    int find_count;
    int value1;
    int value2;
    int value3;
    int value4;
    int calculate_cows;
    string secret_number = "";
    string guess = "";
    bool run_game = true;
    char again;
    bool win;
    cout << "Welcome to the Bulls and Cows game!" << endl;
    while (run_game == true) {
        bull = 0;
        cow = 0;
        find_count = 0;
        calculate_cows = 0;
        //random number generation begin
        srand(time(NULL));
        value1 = rand() % 10;
        value2 = rand() % 10;
        value3 = rand() % 10;
        value4 = rand() % 10;
        // random number generation stop
        stringstream ss;
        ss << value1 << value2 << value3 << value4;
        secret_number = ss.str();
        win = false;
        while (win == false) {
            cout << "Make a guess: (XXXX)" << endl;
            cin >> guess;

            for (int i = 0; i < secret_number.length(); i++) {
                if (guess.at(i) == secret_number.at(i)) {
                    bull++;
                }
            }

            for (int i = 0; i < guess.length(); i++) {
                int secret = secret_number.at(i);
                if (guess.find(secret)) {
                    find_count++;
                }
            }

            calculate_cows = find_count - bull;
            cow = (calculate_cows < 0) ? 0 : calculate_cows;

            cout << "Bulls: " << bull << endl << "Cows: " << cow << endl;
            if (bull == 4) {
                cout << "You win!" << endl;
                win = true;
            }
            else {
                bull = 0;
                cow = 0;
                find_count = 0;
            }
        }
        cout << "Play again? (y/n)" << endl;
        cin >> again;
        run_game = (again == 'y') ? true : false;
    }
    cout << "Thanks for playing! :)" << endl;
    return 0;
}

Prelude

  I've also seen that many in the C++ community seem to favor comments instead of descriptive variable/function names. Example:

int a = 3.14159265359; // pi with 11 decimal numbers

Not really, you should use meaningful names (and not only in C++). pi_eleven_decimal add nothing. Worst, it can be lying or source of trouble. What happen if later you wand to change  to take only 6 digit? You'll leave the lying name, or you'll change all occurrences, opening way to mistakes. The number of digits is an implementation detail which the user don't (have to) care. a isn't even better, you loose completely the sens of your variable. Here, I would name it simply pi.

Also, note that you used the wrong type. You used int so it will be truncated to 3 (plus get some compiler warnings). So, here you can use float pi = 3.14159265359,  double pi = 3.14159265359,  auto pi = 3.14159265359f or   auto pi = 3.14159265359 per example.

Coding idiomatically

Avoid using namespace std;

Putting using namespace std at the top of every program is a bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.

Although it's "safe" to use it in some place (e.g. implementation files), as long as you aren't familiar with the features of c ++, try to avoid it.
It led to a world of name collisions. (best case)
It's source of silent errors and weird bugs. (worst case)
If typing std:: is so tedious for you, try to import only truncated namespaces. ( e.g. using namespace std::string;).
If importing nested namespaces still too awful for you, try to do it inside a restricted scope (e.g. a functions) and not in global scope.

Randomization

Avoid using rand for randomization. Instead use the  <random> header's facilities.  You have a lot of interesting post on SO:

How to generate a random number in C++?
Generate random numbers using C++11 random library
Random number generation in C++11: how to generate, how does it work?

And, if you want a higher randomness, you can try seeding like described here.

But in all case, you don't have to call four times your choses random function. Just call it once and get a number from 0 to 9999.

Then, don't forget to add leading 0's if number is lesser than 1000 (see below *).

Expressions and statements

Declare variables late as possible

Don't declare variables at the top of your functions or before you need it, try to keep their scope as small as possible.  Wait until you have a value to initialize it with.

Always initialize variable (as mush as possible)

Declaring variable and then initialize it much later is a source of mistakes that lead to undefined behavior. Instead, Initialize your variables in the declaration.

Don't be redundant in your conditions

As explained in the Core Guideline, comparing a boolean to true in a condition, not only is useless, but it's also much more verbose.

while (run_game == true) should be while (game_running) (look also at the name change, it's a lot more fluent).

run_game = (again == 'y') ? true : false; should be game_running = (again == 'y');

Input/Output

Don't use std::endl

Using std::endl send a '\n' and then flushes the output buffer. So std::endl is more expensive in performance. Instead, use '\n' and then, if you want to manually flush the stream, explicitly call std::flush.

Sanitize inputs

Don't think users will not try to break your program, they will do, even by mistake. Ensure user give something you expect. Here, you ask for a integer (not a string) and then, only after insuring correctness of the input (lower than 10000 and greater or equal to 0), use std::to_string (or std::stringstream) to convert it to a string. Don't forget to add leading 0's in case where inputed number is lower than 1000. (see below *)

You can also use the same method than linked above for the "play again" input.

Wrap code into functions and factorize

Don't put the code into the main, but instead, move the game's code into its own function.
You are outputting in many place and ask for input then read, in many other place. Try to wrap theses two behaviors into own functions (eg, print and ask_input). it will be easier, if later you want to modify the way you print or read inputs, to just modify one place.
(*) Also, you can have a function (or local lambda) to ensure secret and guess are 4 length, and add padding 0's otherwise.

Choose the right algorithm

Your two for-loop can be rewritten with std::mismatch for the first and  std::set_intersection for the second.

Just after, the cows calculation can be simplified with cow = std::max(0, find_count - bull);

Misc

There are still some refactoring possible, as moving the output "You win!" outside of the loop.

You can also remove the return statement from your main. Quoting C++ Standard :

  If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;