java – Improving Project Euler code problem #10

        int i, k = 3;

I’m simply going to delete this line. In general, it is better to declare variables as close to first use as possible. In this case, inside the loop is possible.

        for(i = 2; i <= 2000000;){

For every candidate number from 2 to 2,000,000. But I can tell you immediately that about half of those won’t be prime. Consider

        outer:
        for (int i = 3; i <= 2000000; i += 2) {

Now you won’t waste your time checking even numbers for primality. There’s only one, so you can just skip along once you include it in the sum (which you already did, good work). I’ll explain more about the outer: label later.

            if (i % 2 == 0){
                i++;
                k = 3;
            }else{

If we’re only checking odd numbers, we don’t have to check if they are even. We can just delete this (and don’t forget the } at the end).

                if (i % k == 0 && i != k){
                    i++;
                    k = 3;
                }else {
                    if (i == k) {
                        sum = (sum + i);
                        i++;
                        k = 3;
                    }else {
                        k = (k + 2);
                    }
                }

Well, looking at it one way, this is rather clever. You increment different variables based on certain conditions. So one for loop effectively loops over two different variables. But from another perspective, it’s wasted cleverness. You’re making things more complicated for no real gain. It would be better to simply nest another for loop here. Why? Well, let’s look at it:

            for (int k = 3; k <= i / k; k += 2) {
                if (i % k == 0) {
                    continue outer;
                }
            }

            sum += i;

So from your twelve lines of code, we drop to seven (eight if we count the blank line, but we usually wouldn’t).

One of the seven lines is the outer: label. Basically that allows us to say that we are done with the inner loop and want to skip the rest of the current iteration of the outer loop. We will resume (or continue) immediately before the increment step or immediately after the body of the loop (same thing; different descriptions). Note that we could make it more readable by moving the logic to check if an odd prime into a separate method. But this is more like your existing solution.

This moves all the management of k into the for loop. Now our check if i is not prime is much simpler. And we don’t have to consider whether or not we want to increment i. We do that every iteration of the outer loop. Nor do we have to think about whether we increment k. We do that every iteration of the inner loop. Your code wasted cleverness on determining whether you wanted to do those things. But nesting the loops does the same thing without cleverness. As such, it is a simpler, more elegant solution.

We also check fewer values here. Note that your loop checked from 2 to i. But we don’t need to check to i. If something is a factor, then there must be a factor that is less than or equal to the square root of i. And k <= i / k is the same as k * k <= i or k <= Math.sqrt(i). It’s the preferred check here for a few reasons.

  1. k * k may overflow the int size. i / k won’t.
  2. In many systems, both i % k and i / k will be calculated at the same time. So since you would next check i % k, the i / k is almost free (not quite since if it fails, you don’t check i % k).
  3. It is usually cheaper (in time) to compute i % k than Math.sqrt(k).

The counter-argument would be that you can calculate Math.sqrt(k) once, prior to the loop and save it in a variable. But as I said, in many systems, you only have to calculate i / k separately once and it will almost always be cheaper than calculating the square root.

Time complexity analysis

Only checking odd numbers won’t affect the complexity analysis. But it may still make things faster.

The larger change is likely to be stopping at the square root. The square root of a million is a thousand. Would you prefer to do something a million times or a thousand? Hopefully obvious that the square root is better.

Separation of concerns

As noted elsewhere, it is often better to separate different concerns. In this case, one concern is generating the primes and another is summing them (and it would be reasonable to think of checking primality as a third). This is true, but I ignored it to concentrate on improvements to the way that you are doing things now. However, please do not take this as an endorsement of mixing concerns like this. That is not usually recommended except in specific cases where optimization demands it. This probably isn’t one of those cases.

For similar reasons, I stuck with testing by trial division rather than using a sieving algorithm. But that doesn’t mean that a sieve wouldn’t solve this problem more quickly.

Improving images for human vision

If human eyes are more sensitive to red, green more than blue, also more sensitive to brightness than color. If there are 24 bits per pixel, are the images modified behind the scenes for human vision? How can we improve image for humans based on this? Or it is not really required? How could we make best of these 24 bits?

c++ – Improving my implementation of a unique_ptr – PPP Stroustrup book

This question is an improvement of this one here: Implementing a unique_ptr – PPP Stroustrup exercise

Here’s my new code, following the suggestions by @JDługosz.

  • I agree with the fact that T* operator is much better than T *operator, but the code formatter of VSCode does this automatically, I’ll try to fix it. Sorry for that.

  • I implemented the move semantics, and testing it seems it’s okay. I’d like to have a check on my implementation as well. In particular, if in the move constructor I write std::move(p.ptr) only, then I have a pointer being freed was not allocated error, and I don’t understand why since after the move, the object from which I “stole” the resource should become a nullptr, but apparently it’s not.

If I write in the definition of that move constructor function p.ptr=nullptr (as I did) it works. Why is that?

#include <iostream>
#include <vector>
#include <cassert>

template <typename T>
class Unique_Ptr
{
private:
    T *ptr;

public:
    explicit Unique_Ptr(T *p = nullptr) noexcept : ptr{p} {}

    //Copy semantics is deleted
    Unique_Ptr(const Unique_Ptr &) = delete;
    Unique_Ptr &operator=(const Unique_Ptr &) = delete;

    //Move semantics
    Unique_Ptr(Unique_Ptr &&p) noexcept : ptr{std::move(p.ptr)}
    {
        p.ptr = nullptr;
    }

    Unique_Ptr &operator=(Unique_Ptr &&p) noexcept
    {
        ptr = std::move(p.ptr);
        p.ptr = nullptr;
        return *this;
    }

    ~Unique_Ptr()
    {
        delete ptr;
    };

c++ – Improving speed and simlify code of “leetcode” Roman Numbers converter

I just have the free account at leetcode. I’ve been practicing some coding there in preparation for interviews. But I can’t see the solution logic behind the IntegerToRoman conversion function. I solved both the leetcode Roman Numbers problems (Arabic -> roman, and roman -> Arabic). I made a solution of my own but its slower than most accepted leetcode solutions.

I would appreciate any feedback code review-wise on the solutions but the main goal is to make them faster somehow. Especially the IntToRoman seemed trickier when I got to coding it. The RomanToInt was tricky as well, but when I realized the reversal it seemed OK. I started to code it with a stack originally instead of the oldval, but variable is enough and stack isn’t needed as such.

The main code:

int fasterRomanToInt(std::string s)
{
    int romanSum{ 0 };
    if (s.size() == 1)
    {
        return oneSizeRomans(s(0));
    }
    else
    {
        int oldval = 0;
        int i = s.length() - 1;
        while (i >= 0)
        {
            char current = s(i);
            int curval = oneSizeRomans(current);
            if (curval >= oldval)
            {
                romanSum += curval;
            }
            else
            {
                romanSum -= curval;
            }
            oldval = curval;
            --i;
        }
        return romanSum;
    }
}


std::string intToRoman(int num)
{
    std::map<int, std::string> ArabicvalueToRomans
    {
        {1000, "M"},
        {900, "CM"},
        {500, "D"},
        {400, "CD"},
        {100, "C"},
        {90, "XC"},
        {50, "L"},
        {40, "XL"},
        {10, "X"},
        {9, "IX"},
        {8, "VIII"},
        {7, "VII"},
        {6, "VI"},
        {5, "V"},
        {4, "IV"},
        {3, "III"},
        {2, "II"},
        {1, "I"}
    };

    std::vector<int> romanSpecials{ 1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 8,7,6,5,4,3,2,1 };

    int arabic = num;
    std::string result = "";
    while (arabic > 0)
    {
        int d = seekLargestPossible(romanSpecials, arabic);
        int r = arabic % d;
        std::string characters = processToRoman(d, ArabicvalueToRomans);
        arabic -= d;
        result += characters;
    }
    return result;
}

Here’s a couple helper functions for intToRoman:

int seekLargestPossible(std::vector<int>& vecRef, int a)
{

    // input bigger than biggest divisor so use 1000
    if (a >= 1000)
    {
        return 1000;
    }



    // seek the biggest divisor that is smallerorequal than the A inputvalue
    
    for (auto iter = vecRef.begin(); iter != vecRef.end(); iter++)
    {
        auto key = *(iter);
        if (key <= a)
        {
            return key;
        }
    }
    throw "something went bad in seekLargestPossible";
}

    std::string processToRoman(int value, std::map<int, std::string>& mapRef)
{
    // if cannot access romanstr in map with key, throws which is good
    // shuldnt be happening tho
    std::string s = mapRef.at(value);
    return s;
}

And here’s a helper function for RomanToInt:

int oneSizeRomans(char c)
{
    /*  I             1
        V             5
        X             10
        L             50
        C             100
        D             500
        M             1000*/
    switch (c)
    {
    case 'I': return 1;
    case 'V': return 5;
    case 'X': return 10;
    case 'L': return 50;
    case 'C':return 100;
    case 'D':return 500;
    case 'M':return 1000;

    default:
        throw "bad things one size romans func";
        break;
    }
}

c++ – Improving poor man’s translations mechanism in c++98 program (follow-up)

There’s no need to set the size of the strings to 16 bytes, just store them as pointers to const char instead of arrays of char:

typedef const char *tr_chars_t(TR_SIZ);

For each language you have a separate variable holding the translations strings, and you have a long if-then-chain to find the right variable given the languages name. You can make the code more generic by storing all languages in an array, like so:

static const tr_chars_t &resolve_lang(const std::string &lang) throw()
{
    static const struct {
        const char *lang;
        const tr_chars_t translations;
    } languages() = {
        {
            "en", {
                "Reload", // TR_RELOAD
                "Save"    // TR_SAVE
            }
        },
        {
            "it", {
                "Ricarica", // TR_RELOAD
                "Salva"     // TR_SAVE
            }
        },
        ...
    };

    static const std::size_t n_languages = sizeof languages / sizeof * languages;

    for (std::size_t i = 0; i < n_languages; ++i) {
        if (lang == languages(i).lang) {
            return languages(i).translations;
        }
    }

    return languages(0).translations; // Fallback lang
}

This way, when adding a language, you only have to add an entry to the array languages(), and not change any other code.

You could even improve it a bit by ensuring the languages are ordered alphabetically, so you can do a binary search.

Copying the C strings in to the array of std::strings can be done with std::copy():

void select_lang(const std::string &lang) throw()
{
    const tr_chars_t &tr_arr = resolve_lang(lang);
    std::copy(tr_arr, tr_arr + TR_SIZ, i_tr);
}

You could also use std::find() or std::lower_bound() to replace the for-loop in my version of resolve_lang(), but it’s a bit awkward in C++98 since you can’t use a lambda for the comparator.

Instead of storing the translation strings in arrays of regular C strings, you can also consider storing them in arrays of std::strings. The drawback is that they will all be initialized once at runtime, but then you can just return a reference to that array, instead of copying the C strings into i_tr. Which method is best depends on how many languages you want to support versus how often you switch languages in your code multiplied by the number of strings in a language.

Improving WordPress coding [closed]

I have some code that I wrote, I think there is room for improvements, but I’m not sure how to go about it, how would you improve each set of code

1)

<section id="<?php echo $section_id; ?>" class="<?php echo $class; ?>">
 <div class="container">
 <h1><?php echo get_sub_field('title'); ?></h1>
 <p><?php echo get_sub_field('paragraph'); ?></p>
 </div>
<?php
$postImage = get_sub_field('post_image');
$siteImage = get_sub_field('site_image');
if($postImage){
 echo "<img src="https://wordpress.stackexchange.com/.$postImage("url').'>";
}
else if($siteImage){
 echo "<img src='.$siteImage('url').'>";
}
else{
 echo "<img src='placeholder.jpg'>";
}
?>
#mainSection, #section_2 {
 display: BLOCK;
 color: #000000;
 background-color: #FFFFFF;
 margin-right: 20PX;
 margin: 0PX;
}
#mainSection { background-color: #A7A7A7; }

❓ASK – Do you think your skill is improving with time? | Proxies-free

As we say, learning is a continuous procees. And no matter how much you learn, the scope of learning will keep on increasing. In forex, the learning is the main pillar. When you learn well, you gain your skills well. Then only you will be able to trade well.

In this learning process, we must continuously gain knowledge and improve our skills. The better our skills, the better will be our profitability in the market. In this forex business, do you think your skills are improving with time? Are you doing efforts to make it more adaptive to market conditions?

❓ASK – Does attending webinars and seminars really help in improving trading skills? | Proxies-free

I for one love attending seminars or workshops, it has a great impact on the listener and opens us to lots of ideas, and help us to avoid mistakes like those people made at their point of career. Seminars help to nurture, train and enlighten beginners the need of seminars and workshops cannot be over emphasize.

Improving regularity of the boundary of a convex set in Riemannian manifolds

Let $X$ be a geodesically complete Riemannian manifold (we may assume that $X$ is simply connected and negatively curved, although I don’t think it matters). Given a closed, convex subset $K subset X$, there is a result by Rolf Walter that the $epsilon$-neighbourhood of $K$ (denoted $K_{epsilon}$) has $C^{1,1}$-regular boundary, i.e. the topological boundary of $K_{epsilon}$ is a $C^{1,1}$-submanifold of $X$. I was wondering if the regularity can be improved if we allow for more deformations of the convex set. Specifically, I would like to know the following:

${bf Question:}$ Given a closed, convex subset $K subset X$ and $epsilon > 0$, does there exist a convex closed $K’$ such that $K subset K’ subset K_{epsilon}$, and $partial K’$ is a $C^2$-submanifold of $X$?}

Considering that Walter’s result goes back to the 70s (Rolf Walter, “Some analytical properties of geodesically convex sets”), this seems like a question whose answer should be known, but I’m struggling to find anything. All methods I could come up with to deform $K$ so that its boundary becomes more smooth while keeping the set convex rely on the second fundamental form, which requires that we make the boundary $C^2$ first.

I should empathize that I do not want to assume that $K$ is compact. (I’m fine assuming that there is a cocompact action by isometries on $K$ though.)