Creating smooth animations using rAF It is my first time trying to create smooth animations using rAF and I would like some insight from more experienced developers. Basically I have 16 elements with x,y coordinates and on mousemove over their parent container, all elements move a set amount in the opposite direction. When I do tests on lower-end machines there is some stutter. Is there any technique I am not aware of that can boost my animation performance?

(function ($) {
$(document).ready(() => {
const animCont = document.querySelector('.shop-header-container');
const animContWidth = animCont.offsetWidth;
const animContHeight = animCont.offsetHeight;
const leftOffset = animCont.getBoundingClientRect().left;
const topOffset = document.querySelector('#primary').offsetTop;
const animImgs = animCont.querySelectorAll('.elem');
let mouseX = 0;
let mouseY = 0;
let mouseOver = false;
let positions = [
  {
    id: 82,
    start: {
      x: 0.596,
      y: 0.654,
    },
    range: 0.4,
  }
...
]
positions.forEach(el => {
  el.target = {};
  el.current = {};
})
function updateElements(){
  mouseTargetX = (mouseX - leftOffset) / animContWidth;
  mouseTargetY = (mouseY + topOffset) / animContHeight;
  positions.forEach((el,i) => {
    if (mouseOver) {
      el.target.x = ((mouseTargetX - el.start.x) * -el.range + el.start.x ) * animContWidth;
      el.target.y = ((mouseTargetY - el.start.y) * -el.range + el.start.y ) * animContHeight;
    } else {
      el.target.x = el.start.x * animContWidth;
      el.target.y = el.start.y * animContHeight;
    }

    if(!el.current.x) {
      el.current.x = el.target.x;
      el.current.y = el.target.y;
    } else {
      el.current.x = el.current.x + (el.target.x - el.current.x)*0.1;
      el.current.y = el.current.y + (el.target.y - el.current.y)*0.1;
    }

    animImgs[i].style.transform = 'translate3d('+el.current.x+'px,'+el.current.y+'px,0)';
  })
  requestAnimationFrame(updateElements);
}
updateElements();
function animContMouseEnter(e){
  mouseOver = true;
}
function animContMouseLeave(){
  mouseOver = false;
}
function animContMouseOver(e){
  mouseX = e.clientX;
  mouseY = e.clientY;
}
animCont.addEventListener("mousemove",animContMouseOver,false);
animCont.addEventListener("mouseenter",animContMouseEnter,false);
animCont.addEventListener("mouseleave",animContMouseLeave,false);
});
}(jQuery));

There is not enough information in your question to get a clear picture of where there may be a problem.

Your code

You code does not have any glaring performance issues. Some minor changes would be...

Use for loops rather than forEach. (Minor performance increase but when animating its always good to use the fastest style)
Don't use 3D transforms if you are only doing 2D animation. Better yet, if only 2d dont use the transform ate all. Set thestyle.positiontoabsoluteand use thetop,left` CSS style attributes to set position.
You only use jQuery to get the onload event. Its just bloat so don't load it if you don't need it.

Animating elements.

Some points related to animating elements.

Its not just the elements that you are animating that add to the total load of an animation. Other elements may require rendering (compositing) and they may also be triggering reflow/s. Composites and reflows are expensive operations

The order of the animated elements in relationship to other content can have a big impact on performance due to inefficient compositing. For example you may be animating items A and B, and on the page you have c, A, d, B, e (where c,d,e are other elements) . In the worst case the compositing of the content will be as follows... render c, composite c with background, render A, composite A with previous composite, render b, composite b with previous composite, and so on.

So you just change and move 2 items but that may trigger 5 renders and 5 composites, and depending on the layout, they may be the 5 full screen composites and renders each frame.

Are you using filters? Depending on the filters (and the GPU's abilities) they can add to the workload. Use filters sparingly, don't make more than one change to filters during a frame.

Can all the image and pixel related content fit into GPU RAM. If not reduce the number of pixels. Low end devices can have very low amounts of GPU RAM and if you exceed the capacity of the GPU the browser will swap it in and out of CPU RAM as needed. This is SLOW... very SLOW...

Take charge of the rendering.

It can be very difficult to locate animation bottle necks when animating elements.

One way to remove the rendering/compositing ambiguity is to use the canvas. You then get to efficiently control the rendering compositing process. The canvas 2D and WebGL API's use the same code (there abouts) as the browsers rendering and compositing systems.

With some minor changes your animation items can be rendered on a canvas, needing only one render pass and one composite (if you also render the background you can eliminate composite)

The snippet is an example of canvas rendering performance. There are 500 bugs
each rendered with a full transform scale, position, and rotated. This you would never be able to do using HTML elements.

requestAnimationFrame(update); // Start animation when ready

const image = new Image;
image.src = "https://i.stack.imgur.com/C7qq2.png?s=328&g=1";
const canvas = document.createElement("canvas");
var ctx = canvas.getContext("2d");
canvas.style.position = "absolute";
canvas.style.top = "0px";
canvas.style.left = "0px";
document.body.appendChild(canvas);

const mouse  = {x : 0, y : 0}
document.addEventListener("mousemove", (e) => {
 mouse.x = e.pageX;
 mouse.y = e.pageY;
});

var w,h;
function resize(){
    w = canvas.width = innerWidth;
    h = canvas.height = innerHeight;
}
resize();
addEventListener("resize",resize);
const rand  = (min = 1, max = min + (min = 0)) => Math.random() * (max - min) + min;
const DO = (count,callback) => { while (count--) { callback(count) } }
const sprites = [];
DO(500,()=>{
    sprites.push({
       x : rand(w),
       y : rand(h),
       xr : 0,
       yr : 0, // actual position of sprite
       r : rand(Math.PI * 2),
       scale : rand(0.1,0.25),
       dx : rand(-2,2),
       dy : rand(-2,2),
       dr : rand(-0.1,0.1),
    });
});

// Draws a sprite.
function drawImage(image, spr){
    ctx.setTransform(spr.scale, 0, 0, spr.scale, spr.xr, spr.yr); // sets scales and origin
    ctx.rotate(spr.r);
    ctx.drawImage(image, -image.width / 2, -image.height / 2);
}

// Main animation loop
function update(time){
    var ihM,iwM,dx,dy,spr,i,iw,ih,dist;
    ctx.setTransform(1,0,0,1,0,0);
    ctx.clearRect(0,0,w,h);
    const runHalf = time / 3000;
    if(image.complete){
      iw = image.width;
      ih = image.height;
      const size = (iw * iw + ih * ih) / 2;
      for(i = 0; i < sprites.length; i ++){
          spr = sprites[i];
          dx = mouse.x - spr.xr;
          dy = mouse.y - spr.yr;
          dist = dx * dx + dy * dy;
          if(dist/ spr.scale < size){
              const dir = -Math.atan2(dy,dx) -0.2 + rand(0.4);
              const speed = rand(18);
              spr.dx += Math.cos(dir) * speed;
              spr.dy += Math.sin(dir) * speed;

          }else if(((i + runHalf) | 0) % 2){ // half run away
              spr.dx += dx * 0.001 + Math.cos(spr.r) * 0.1;
              spr.dy += dy * 0.001 + Math.sin(spr.r) * 0.1;
          }else{
              spr.dx += -dx * 0.001 + Math.cos(spr.r) * 0.1;
              spr.dy += -dy * 0.001 + Math.sin(spr.r) * 0.1;
          }

          spr.dx *= 0.99;
          spr.dy *= 0.99;
          // limit speed
          dist = spr.dx * spr.dx + spr.dy * spr.dy;
          if(dist > 54){
              spr.dx *= 0.9;
              spr.dy *= 0.9;
          }

          spr.x += spr.dx;
          spr.y += spr.dy;
          spr.r += spr.dr;
          iwM = iw * spr.scale * 2 + w;
          ihM = ih * spr.scale * 2 + h;
          spr.xr = ((spr.x % iwM) + iwM) % iwM - iw * spr.scale;
          spr.yr = ((spr.y % ihM) + ihM) % ihM - ih * spr.scale;
          drawImage(image,spr);
      }
    }
    requestAnimationFrame(update);
}


Takes in two sorted lists and outputs a sorted list that is their union Write a function that takes in two sorted lists and outputs a sorted
  list that is their union

Could anyone help me to improve this code and make it more user-friendly? We are not supposed to use any built-in Python functions, but rather develop our own efficient algorithms.

def answer(list1, list2):

    len1 = len(list1)
    len2 = len(list2)

    final_list = []
    j = 0
    k = 0

    for i in range(len1+len2):

        if k == len1:
            final_list.append(list2[j])
            j += 1
            continue

        if j == len2:
            final_list.append(list1[k])
            k += 1
            continue

        if list1[k] < list2[j]:
            final_list.append(list1[k])
            k += 1

        else:
            final_list.append(list2[j])
            j += 1

    return final_list

print answer([1, 2 , 17, 18, 100], [3, 3, 4, 5, 15, 19, 20, 101])

I think your solution is ok.

Instead of using continue, consider using if..elif..elif..else:

if k >= len1:
  ...
elif j >= len2:
  ...
elif ...:
  ...
else:
  ...

Also, the test k >= len1 is an example of "defensive programming". It protects against k skipping over len1 when being incremented in the loop. Also, the expression list1[k] doesn't make sense unless k < len1, and the negation of that is k >= len1 which is another good reason to use it in the if statement.

If you've learned about xrange() then I would use that instead of range() -- see these SO answers:

https://stackoverflow.com/questions/135041/should-you-always-favor-xrange-over-range

Otherwise it looks good!

We are not supposed to use any built-in Python functions, but rather develop our own efficient algorithms.

You are not following the spec. len is a built-in function, range is a built-in class with many hidden functions, + is implemented as a hidden function, as > and < are. The only way to avoid using any built-in Python functions is not using Python.

The best solution to many problems is using as many Python built-ins as possible. The solution to this problem is simply:

def merge(list1, list2):
    return sorted(list1 + list2)

This solution is way faster than yours, and the new list is now lazy, saving memory. Ironically, my function, that uses only 2 built-in functions (sorted and +) follows the spec more than yours that uses tens of such built-in functions.


Function that recursively downloads entire directory is slow I created this function to recursively copy an entire directory from an FTP server. It works just fine except that it is about 4 times slower than using FileZilla to do the same operation. It takes approximately 55 seconds to download the directory in FileZilla but it takes 229 seconds with this function. What can I do to make it download/run faster?

Private Sub CopyEntireDirectory(ByVal directory As String)
    Dim localPath = localDirectory & formatPath(directory)
    'creates directory in destination path
    IO.Directory.CreateDirectory(localPath)

    'Gets the directory details so I can separate folders from files
    Dim fileList As ArrayList = Ftp.ListDirectoryDetails(directory, "")

    For Each item In fileList
        'checks if it's a folder or file: d=folder
        If (item.ToString().StartsWith("d")) Then
            'gets the directory from the details
            Dim subDirectory As String = item.ToString().Substring(item.ToString().LastIndexOf(" ") + 1)
            CopyEntireDirectory(directory & "/" & subDirectory)
        Else
            Dim remoteFilePath As String = directory & "/" & item.ToString().Substring(item.ToString().LastIndexOf(" ") + 1)
            Dim destinationPath = localPath & "\" & item.ToString().Substring(item.ToString().LastIndexOf(" ") + 1)
            'downloads file to destination directory
            Ftp.DownLoadFile(remoteFilePath, destinationPath)
        End If
    Next
End Sub

This function below is what is actually taking up 97% of the time.

Public Sub DownLoadFile(ByVal fromFilename As String, ByVal toFilename As String)
    Dim files As ArrayList = Me.ListDirectory(fromFilename, "")
    Dim request As FtpWebRequest = Me.CreateRequestObject(fromFilename)
    request.Method = WebRequestMethods.Ftp.DownloadFile

    Dim response As FtpWebResponse = CType(request.GetResponse(), FtpWebResponse)
    If response.StatusCode <> FtpStatusCode.OpeningData AndAlso response.StatusCode <> FtpStatusCode.DataAlreadyOpen Then
        Throw New ApplicationException(Me.BuildCustomFtpErrorMessage(request, response))
    End If

    Dim fromFilenameStream As Stream = response.GetResponseStream()
    Dim toFilenameStream As FileStream = File.Create(toFilename)

    Dim buffer(BLOCK_SIZE) As Byte
    Dim bytesRead As Integer = fromFilenameStream.Read(buffer, 0, buffer.Length)
    Do While bytesRead > 0
        toFilenameStream.Write(buffer, 0, bytesRead)
        Array.Clear(buffer, 0, buffer.Length)
        bytesRead = fromFilenameStream.Read(buffer, 0, buffer.Length)
    Loop

    response.Close()
    fromFilenameStream.Close()
    toFilenameStream.Close()
End Sub

Check your buffer/BLOCKSIZE. If it's too small, it may be making many more reads and writes, and each has to wait for the network to respond.

I'm not given any information as to what block size you're using, so I can't help anymore.


Recursively reading a directory in node.js I made a function for recursively reading a directory and it just seems kind of long and complicated. If you could take a look and tell me what's good or bad about it, that would be great.

Reading the directory synchronously is not an option.

var dutils = require('./dutils');
module.exports = {
    getIndex: function(ignore, cb) {
        var root = this.root;
        var cbs = 1;
        var index = {};

        // recursive function for walking directory
        (function walk(dir) {
            fs.readdir(dir, function(err, files) {
                if (err) return cb(err);

                files.forEach(function(fileName) {
                    // prepare
                    var filePath = path.join(dir, fileName); // filePath with root
                    var relFilePath = path.relative(root, filePath); // filePath without root

                    // if the file is on our ignore list don't index it
                    if (dutils.globmatch(filePath, ignore)) return;

                    var stats = fs.statSync(filePath);
                    var isDir = stats.isDirectory();

                    index[relFilePath] = {
                        isDir: isDir,
                        mtime: stats.mtime
                    };

                    // if the file is a directory, walk it next
                    if (isDir) {
                        // count the number of callbacks that are running concurrently
                        cbs++;
                        walk(filePath);
                    }
                });

                // our callback was completed
                cbs--;

                // if all callbacks were completed, we've completely walked our directory
                if (cbs === 0) cb(null, index);
            });
        })(root);
    }
}

My comments are inline starting with the //*. I also very much encourage you to look at async libraries such as Q.

var dutils = require('./dutils');

//* Why export an object with a single function? Why not just export the function itself?
module.exports = {
    //* I don't like intermixing method implementation with exports declaration because you tend to be
    //* looking for very different things when looking at each. Hoisting to the rescure!
    //*     module.exports = { getIndex: getIndex };
    //*     ... later ...
    //*     function getIndex(ignore, cb) {
    //*         ...
    getIndex: function(ignore, cb) {
        //* What if they didn't pass a  callback? Might as well check here and throw an error. Otherwise
        //* there will be a confusing error later on

        //* What is `this`? I **highly** recommend not using the `this` parameter unless you
        //* understand _very_ well what it does and how it works. You can almost always achieve the same
        //* thing using simpler techniques
        var root = this.root;
        //* cds is not a good variable name - I have no idea what this represents reading it
        var cbs = 1;
        var index = {};

        // recursive function for walking directory
        (function walk(dir) {

            //* I usually prefer naming callback functions. Not only does it allow you to give a good
            //* label to the code but the function will show up labeled in stack traces
            fs.readdir(dir, function(err, files) {
                //* Be aware that it is possible for the callback to be triggered multiple times when there are
                //* errors. This is an unusual interface and just be certain that you do this on purpose
                if (err) return cb(err);

                //* Again, I would recommend labeling this function.
                //* Also I'm not sure if this will follow symlinks or not but if does then this might end up in
                //* an endless loop.
                files.forEach(function(fileName) {
                    // prepare
                    var filePath = path.join(dir, fileName); // filePath with root
                    var relFilePath = path.relative(root, filePath); // filePath without root

                    //* I don't think this is really a useful comment as it doesn't say much more than
                    //* the code itself does. I also usually prefer putting the `return` statement for early returns
                    //* on its own line to make the early return stand out visually.
                    // if the file is on our ignore list don't index it
                    if (dutils.globmatch(filePath, ignore)) return;

                    var stats = fs.statSync(filePath);
                    var isDir = stats.isDirectory();

                    index[relFilePath] = {
                        isDir: isDir,
                        mtime: stats.mtime
                    };

                    //* This comment is not very useful as it - again - just restates what the code does
                    // if the file is a directory, walk it next
                    if (isDir) {
                        // count the number of callbacks that are running concurrently
                        cbs++;
                        walk(filePath);
                    }
                });

                // our callback was completed
                cbs--;

                // if all callbacks were completed, we've completely walked our directory
                if (cbs === 0) cb(null, index);

                //* Is the entire point of the `cbs` counter to see if any `readdir` operation resulted in an error?
                //* that seems to be all that it does. Why not just have a `var lastError = null` in `getIndex` and
                //* rather than checking for err do `if(lastError = err) return`. That way if `err` is truthy you have
                //* an error, otherwise you do not
                //*
                //* Also keep in mind that cb will be called multiple times but each time with the
                //* same instance of the index object. That means that previously invoked callbacks will be
                //* having this object mutate underneat them. This might be exactly what you want but it is unuaual
                //* and should definitely be documented clearly. At the very least you want a big bold doc comment
                //* on this function
            });
        })(root);
    }
}

Here is an article I wrote on this parameters

It's more efficient, better to maintain and the code becomes much cleaner if you use the promisify native Node function to transform the fs callback functions to promise based functions, then write your code with async/await style.

Code:

const fs = require('fs');
const { promisify } = require('util');
const path = require('path');
const readDirAsync = promisify(fs.readdir);
//
const sourcePath = process.argv[2] || './';
//
(async () => {

    // Main execution
    var finalRes = await scanDir(sourcePath);
    console.log('Scan result', finalRes);
    process.exit(0);

    // Recursive async function
    async function scanDir(dir) {
        const list = await readDirAsync(dir);
        // Array.map returns promisses because the argument function is async
        return Promise.all(list.map(async (name) => {
            try {
                // File (or directory) info
                const itemPath = path.join(dir, name);
                const stats = fs.statSync(itemPath);
                const isDir = stats.isDirectory();
                var children = null;
                // Recursion for directories
                if (isDir) {
                    children = await scanDir(itemPath);
                }
                //
                return {
                    name,
                    itemPath,
                    isDir,
                    parent: dir,
                    children
                };
            }
            catch (err) {
                console.error('Error in scanDir()', err);
            }
        }));
    }
})();


Find index of the nearest larger number of a number The task:

  Given an array of numbers and an index i, return the index of the
  nearest larger number of the number at index i, where distance is
  measured in array indices.

  For example, given [4, 1, 3, 5, 6] and index 0, you should return 3.

  If two distances to larger numbers are the equal, then return any one
  of them. If the array at i doesn't have a nearest larger integer, then
  return null.

  Follow-up: If you can preprocess the array, can you do this in
  constant time?

My solution:

const nearestLargestNumber = (arr, idx1) => {
  const num = arr[idx1];
  let nearestLargest;
  const findIdxOfNearestLargest = (idxOfNearestLargest, item, idx2) => {
    if (item > num &&
       (idxOfNearestLargest === null || nearestLargest > item )) {
      nearestLargest = item;
      idxOfNearestLargest = idx2;
    }
    return idxOfNearestLargest;
  };

  return arr.reduce(findIdxOfNearestLargest, null);
};

console.log(nearestLargestNumber([4, 1, 3, 5, 6], 0));

EDIT: I mistook "nearest" to mean "numerically closest" but the problem asks for minimum distance from the index.  As the other answer points out, there are faster methods than visiting every element.

The answer to the followup question is "yes."

"Larger" is ambiguous: if you ask someone for a "large negative number" they probably don't answer "negative one."   Attribute this to sloppiness on the author's part, unless you've reason to suspect trickery.

I'm not a fan of these variable names: idx1 should be idx0, or i0 or initialIndex.  nearestLargest isn't the largest anything and nearestLarger would be more accurate.  num is a number, but so is everything else—maybe minimum or baseline or mustBeGreaterThan. nearestLargestNumber contains an index, not a number, while the number is held in nearestLargest.

I suggest one common prefix for your index variables and another prefix for value variables (though you don't really need more than one).

const nearestGreater = (arr, i0) => arr.reduce(
        (iNearestGreater, val, i) =>
            ( val > arr[i0] && ( iNearestGreater===null || val < arr[ iNearestGreater ] ) ) ?
                i : iNearestGreater,
        null
    );

Complexity: \$O(\log n)\$
It worked. It gave me correct answers for many testcases:
i/p:[1, 5, 4] ,0
          o/p:1

i/p:[4, 1, 3, 5, 6] ,0
         o/p:3

l=list(map(int,input().split()))
n=int(input())
p=n-1
q=n+1
while(p>=0 or q<len(l)):
    if(p>=0 and l[p]>l[n]):
        print(p)
        break
    elif(q<len(l) and l[q]>l[n]):
        print(q)
        break
    p=p-1
    q=q+1
if(p<0 and q>=len(l)):
    print("null")


Match formula for loop in VBA I am using a match formula inside of a for loop, but it is taking too much time. Is there any way to optimize this code to make it go faster?

 Lastrow = ActiveSheet.UsedRange.SpecialCells(xlCellTypeLastCell).Row
For i = 3 To Lastrow

       Sheets("sample").Range("AM1000000").End(xlUp).Offset(1, 0).Select
        Selection.FormulaArray = _
        "=IF(ISNUMBER(MATCH(1," & Chr(10) & "  (order!R2C15:R1000000C15=RC[-24])*" & Chr(10) & "  (order!R2C7:R1000000C7=RC[-32])*" & Chr(10) & "  (order!R2C24:R1000000C24=RC[-15])," & Chr(10) & "  0)), ""pass"",""review"")"

    Next i
      Columns("AM:AM").Select
    Selection.Copy
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False

The key optimisation can be found in this part:

Dim orderColumnOArray as Variant, orderColumnGArray as Variant, orderColumnXArray as Variant
orderColumnOArray = ThisWorkbook.Worksheets("order").Range("O2:O1000000").Value
orderColumnGArray = ThisWorkbook.Worksheets("order").Range("G2:G1000000").Value
orderColumnXArray = ThisWorkbook.Worksheets("order").Range("X2:X1000000").Value

There above, we are using arrays to store the order data, and we can traverse them more efficiently, rather than repeat a full array formula calculation for each cell.

Note that even then we could cut down on the million rows, and do a lastRow calculation for the order sheet. Perhaps that will bring great improvement too. However if we say for now that you will genuinely have that number of data rows in the order Worksheet, we can still go about it a bit better.

With ThisWorkbook.WorkSheets("sample")
    'I will assume that the column needs to filled from AM3 to AMLastrow
    'I also assume the tested values are strings or integers, not dates or floats
    Dim resultArray() as String
    ReDim resultArray(3 to Lastrow)
    Dim j as Long

    For i = 3 To Lastrow
        resultArray(i) = "review"
        For j = 1 To Ubound(orderColumnOArray)
            'The below nested Ifs can be done in many different ways
            'e.g. And with the 3 equality tests
            'or even by initially making two concatenated arrays!
            If .Range("O" & i).Value = orderColumnOArray(j, 1) Then
                If .Range("G" & i).Value = orderColumnGArray(j, 1) Then
                    If .Range("X" & i).Value = orderColumnXArray(j, 1) Then
                        resultArray(i) = "pass"
                        Exit For
                    End if
                End if
            End if
        Next j
    Next i

    'Now we can write back the array to the worksheet faster
    .Range("AM3").Resize(Lastrow - 3 + 1, 1).Value = resultArray
End With

May want to read the cells into an array, process within the VBA world, then write out cells to the sheet. Going back and forth to the worksheets typically slow.


Merge two lists in Java and sort them using Object property and another condition I have two lists List<Person> list1 = new ArrayList(), list2 = new ArrayList(); (Not the same size), of the class Person:

public class Person {
    private String name;
    private int age;

    public Person(String name, int age) {
        this.name = name;
        this.age = age;
    }

    public String getName() {
        return name;
    }

    public int getAge() {
        return age;
    }

    @Override
    public String toString() {
        return "Person{" + "name=" + name + ", age=" + age + '}';
    }

}

I want to create a new list using list1 and list2 sorted by age (descending), but I also another condition that is better explained with an example:

//L1 = from list1, L2 = from list2
//looping trough the new list
Person{name=Alec L1, age=75}
Person{name=Menard L1, age=50} //Bob should be here
Person{name=Bob L2, age=50}

He should, because his age is equal to Menard, Alec is from L1 and two Person from L1 can't be one after another is this kind of situation happens.

A complete list should look like this:

Person{name=Giant L2, age=100}
Person{name=Derp L1, age=50}
Person{name=John L2, age=50}
Person{name=Menard L1, age=44}
Person{name=Lili L2, age=44}
Person{name=Lili L1, age=44}
Person{name=Menard L2, age=44}
Person{name=Bob L1, age=22}
Person{name=Alec L2, age=21}
Person{name=Alec L1, age=21}
Person{name=Herp L2, age=21}
Person{name=Herp L1, age=21}
Person{name=Alice L1, age=12}
Person{name=Little L2, age=5}

Here is my complete code to achieve this result:

    List<Person> list1 = new ArrayList(), list2 = new ArrayList();

    list1.add(new Person("Derp L1", 50));
    list1.add(new Person("Alec L1", 21));
    list1.add(new Person("Herp L1", 21));
    list1.add(new Person("Menard L1", 44));
    list1.add(new Person("Lili L1", 44));
    list1.add(new Person("Alice L1", 12));
    list1.add(new Person("Bob L1", 22));

    Collections.sort(list1, new Comparator<Person>() {
        @Override
        public int compare(final Person object1, final Person object2) {
            return object2.getAge()-object1.getAge();
        }
    });

    list2.add(new Person("Little L2", 5));
    list2.add(new Person("Lili L2", 44));
    list2.add(new Person("Alec L2", 21));
    list2.add(new Person("Herp L2", 21));
    list2.add(new Person("Menard L2", 44));
    list2.add(new Person("Giant L2", 100));
    list2.add(new Person("John L2", 50));

    Collections.sort(list2, new Comparator<Person>() {
        @Override
        public int compare(final Person object1, final Person object2) {
            return object2.getAge()-object1.getAge();
        }
    });
    List<Person> allList = new ArrayList();
    int l1 = 0, l2 = 0;
    for (int i = 0; i < list1.size()+list2.size(); i++)
        if (l1 < list1.size() && l2 < list2.size())
            if (list1.get(l1).getAge() > list2.get(l2).getAge())
                allList.add(list1.get(l1++));
            else if (list1.get(l1).getAge() < list2.get(l2).getAge())
                allList.add(list2.get(l2++));
            else if (allList.size() > 0)
            {
                if (list1.get(l1).getName().contains("L1") == allList.get(allList.size()-1).getName().contains("L1"))
                    allList.add(list2.get(l2++));
                else
                    allList.add(list1.get(l1++));
            }
            else
                allList.add(list1.get(l1++));
        else if (list1.size() == l1)
            allList.add(list2.get(l2++));
        else
            allList.add(list1.get(l1++));

    for (Person person : allList)
        System.out.println(person.toString());

But, is there another way to do it? More elegant code or using some built in Java class?

Your problem statement is not very clear.  As I understand it, you want to have a combined sorted list but interleave elements from list1 and list2 whenever the age is the same.

Here's a simple implementation of that logic.  I used java 8 streams to sort lists and put them in ArrayDeques.

import static java.util.stream.Collectors.toCollection;
import static java.util.Comparator.comparingInt;

static<T> List<T> interleaveSort(List<T> list1, List<T> list2, Comparator<T> cmp) {
    // sort the lists and set them up as queues
    Deque<T> x = list1.stream().sorted(cmp).collect(toCollection(ArrayDeque::new));
    Deque<T> y = list2.stream().sorted(cmp).collect(toCollection(ArrayDeque::new));

    List<T> result = new ArrayList<>(x.size() + y.size());

    while (!x.isEmpty() && !y.isEmpty()) {
        if (cmp.compare(x.peek(), y.peek()) > 0) {
            result.add(y.poll());
        } else {
            // whenever we pick from X, swap queues so next time we will favor the other list
            result.add(x.poll());
            Deque<T> t = x;
            x = y;
            y = t;
        }
    }
    // since the while loop terminates when one of the queues is empty
    // we should add the left-over elements
    result.addAll(x);
    result.addAll(y);

    return result;
}

Then, in your main method:

List<Person> allList=interleaveSort(list1, list2, comparing(Person::getAge).reversed());

You can do list1.addAll(list2) and then sort list1 which now contains both lists


Converting UK drams to ounces and pounds in PHP I have written this little function to convert UK weight drams (dr) to ounces (oz) and pounds (lbs). As far as my research goes, 1 lbs = 16 oz and 1 oz = 16 dr. I need the function to return an array of lbs, oz and dr.

Am I doing it right? Or is there a better way to do it? Has my method got any flaws?

function fromDrams($drams){

    // Nullify pounds
    $ouncesAndDrams = $drams % 256;

    $lbs = (int) ($drams / 256);
    $oz  = (int) ($ouncesAndDrams / 16);
    $dr  = $ouncesAndDrams % 16;

    return ['lbs' => $lbs, 'oz' => $oz, 'dr' => $dr];
}

It does return good values when tested:

fromDrams(273) = ['lbs' => 1, 'oz' => 1, 'dr' => 1];
fromDrams(272) = ['lbs' => 1, 'oz' => 1, 'dr' => 0];
fromDrams(271) = ['lbs' => 1, 'oz' => 0, 'dr' => 15];
fromDrams(256) = ['lbs' => 1, 'oz' => 0, 'dr' => 0];
fromDrams(32) = ['lbs' => 0, 'oz' => 2, 'dr' => 0];
fromDrams(17) = ['lbs' => 0, 'oz' => 1, 'dr' => 1];
fromDrams(15) = ['lbs' => 0, 'oz' => 0, 'dr' => 15];

There are two things that stand out to me in your code. The first is that your integer cast seems a bit premature. When creating a general function like this, you probably do not want to round the answer, since 1.5 oz is a perfectly valid weight. A user of your function can then cast the resulting values if this is required.

The second is that your conversion values are magic numbers. These should be in constants with names that express the meaning of the number.

A class would be perfect to store the logic and constants. A Weight class would probably be a well-suited name. An Weight object can then hold the amount of weight in one specific unit and provide public methods to retrieve the weight in other units.

The Weight class that I have created here provides three static "Builder/Factory" methods to create a Weight object from. The constructor has been made private to support the use of the more expressive static function. The reasoning behind this, is that $weight = Weight::fromDrams(16); is a lot easier to understand than $weight = new Weight(16);.

Furthermore, the class provides the three public methods to retrieve the given weight for a specific unit.

class Weight {
    const POUNDS_TO_OUNZES = 16;
    const OUNZES_TO_DRAMS = 16;

    /**
     * @var float
     */
    private $drams;

    public static function fromPounds ($pounds) {
        return self::fromOunzes($pounds * self::POUNDS_TO_OUNZES);
    }

    public static function fromOunzes ($ounzes) {
        return self::fromDrams($ounzes * self::OUNZES_TO_DRAMS);
    }

    public static function fromDrams ($drams) {
        return new static($drams);
    }

    private function __construct ($drams) {
        $this->drams = $drams;
    }

    public function inPounds () {
        return $this->inOunzes() / self::POUNDS_TO_OUNZES;
    }

    public function inOunzes () {
        return $this->inDrams() / self::OUNZES_TO_DRAMS;
    }

    public function inDrams () {
        return $this->drams;
    }
}

If you still require a very specific function that returns an array of 'lbs', 'oz' and 'dr' like your question says, than this Weight class can be used in conjunction with it as such:

function fromDrams ($drams) {
    $weight = Weight::fromDrams($drams));

    return [
        'lbs' => (int) $weight->inPounds(),
        'oz' => (int) $weight->inOunzes(),
        'dr' => (int) $weight->inDrams(),
    ];
}

Here you see the Weight class not performing any rounding itself and letting the user of the class perform these when required.

This Weight class is now much more versatile than your original piece of code and can be used in many other situations. For example a website that provides conversion of an amount of weight for a specific unit to the respective weights for the other three units.

if (!empty($_POST["amount_in_drams"]))
    $weight = Weight::fromDrams($_POST["amount_in_drams"]);

if (!empty($_POST["amount_in_ounzes"]))
    $weight = Weight::fromOunzes($_POST["amount_in_ounzes"]);

if (!empty($_POST["amount_in_pounds"]))
    $weight = Weight::fromPounds($_POST["amount_in_pounds"]);

echo $weight->getPounds() . " pounds, ";
echo $weight->getOunzes() . " ounzes or ";
echo $weight->getDrams() . " drams.";

I'm not convinced your method of type casting to (int) is the most reliable way to receive output. That's probably just a personal preference, but other than that I can't see any dramatic ways to improve your code.

Fundamentally, I spent a few minutes figuring out what the output represented. It's seems strange to convert one unit into two units (a new unit plus the original one). Here in the US, I've seen ounces expressed with a decimal, so you might consider just keeping your code metric -> imperial (not mixed).

A possible alternative to what you have:

Use a while loop and count up for each unit. This method is often seen is change dispenser scripts (Out of $1.74, how many quarters, nickels, pennies, etc.).

Simply declare a a variable pounds, and then in a while loop, count up $pounds++; until you cannot make a whole pound any longer.

E.g.:

$pounds = 0;

while ($drams/256 > 1) {
  $pounds++;
  $drams -= 256;
}

This results in your $pounds and then $drams/16 is ounces, optionally roundable. If keeps drams in your output is necessary, simply repeat the above code but in respect to ounces. What's let over is drams!

I hope you're alright with receiving this alternative, instead of heavy review of what you have. But in my opinion, type cast can cause you to lose some of the precision that is lost when moving to integers.


Lattice path from Project Euler with Python solution I was trying to solve this problem called lattice path from Project Euler:

  Count the number of unique paths to travel from the top left to the bottom right of a lattice of squares.

How many such routes are there through a 20×20 grid? It takes me forever to get an answer...

(source: projecteuler.net)

    #  Input:     An interger N (which is the size of the lattice)
    #  Output:    An interger (which represents the number of unique paths)
    #
    #  Example:   input: 2
    #
    #             (2 x 2 lattice of squares)
    #              __ __
    #             |__|__|
    #             |__|__|
    #

When moving through the lattice, you can only move either down or to the right.

I currently have 1 solution, and I am not sure how much better if we memoize it. But I wanted to ask for code review for my solution below:

def lattice_paths(m, n):
    if m == 0 and n == 0:
        return 1
    if m < 0 or n < 0:
        return 0
    return lattice_paths(m - 1, n) + lattice_paths(m, n - 1)

Here is the time complexity and space complexity of my first solution.
# Time Complexity: O(2^(M+N))
# Auxiliary Space Complexity: O(M+N)

Dynamic approach with the memorized solution:

def lattice_paths(m, n): # m rows and n columns

    def helper(x, y):

      if x > helper.m or y > helper.n:
        return 0
      if helper.cache.get((x, y)) is not None:
        return helper.cache[(x, y)]

      helper.cache[(x, y)] = helper(x + 1, y) + helper(x, y + 1)
      return helper.cache[(x, y)]
    helper.cache = {}

    helper.m = m
    helper.n = n
    helper(0,0)

    if m  ==  0 and n == 0:
      return 1
    elif m < 0 or n < 0:
      return 0
    else:
      return helper.cache[(0, 0)]

All of your approaches are very brute force.  The answer can be computed with a bit of mathematical reasoning.  For an m × n lattice, all paths will have m + n segments, of which m segments will go down and n segments will go right.  So, out of m + n slots, count the ways to pick m downward moves; the rest of them will be rightward moves.  The number of paths is:

$${m + n \choose m} = {m + n \choose n} = \frac{(m+n)!}{m! \, n!}$$

So, the Python solution is simply:

from math import factorial

def lattice_paths(m, n):
    return factorial(m + n) // factorial(m) // factorial(n)

Even though the calculation involves large factorials, Python supports big integers seamlessly, and arrives at the answer nearly instantaneously.

here's a solution that uses cache to memoize the solution

def lattice_paths(m, n):
    cache = [1]
    larger = max(m, n)
    smaller = min(m, n)
    while(len(cache) < larger + 1):
        for i in range(1, len(cache)):
            cache[i] += cache[i - 1]
        cache.append(2 * cache[len(cache) - 1])
    return cache[smaller]

# Time Complexity: O(N)
# Auxiliary Space Complexity: O(N)


Random alphanumeric password generator with GOTOs I was inspired after this thread on Stack Overflow to create a random 8 character alphanumeric password generator.

Sadly, it is closed, so I cannot provide an answer there. Anyway, here is the code. Let me know if there are any bugs/bias going on.

using System.Collections.Generic;
using System.Security.Cryptography;

public class AlphanumericGen
{
    public virtual string Generate(byte length)
    {
        ICollection<char> chars = new LinkedList<char>();
        var buffer = new byte[1];
        short counter = 0;
        using (var rng = new RNGCryptoServiceProvider())
        {
            while (counter < length)
            {
                rng.GetBytes(buffer);
                var nextChar = (char)buffer[0];

                if (nextChar >= '0' && nextChar <= '9')
                {
                    goto addChar;
                }

                if (nextChar >= 'A' && nextChar <= 'Z')
                {
                    goto addChar;
                }

                if (nextChar >= 'a' && nextChar <= 'z')
                {
                    goto addChar;
                }

                continue;

            addChar:
                chars.Add(nextChar);
                ++counter;
            }
        }

        return string.Concat(chars);
    }
}

As a general rule of thumb, any time you feel the need to use goto, take a couple of aspirin and lay down until the feeling passes.  They probably should have been deprecated decades ago.

In this particular case, using a string of allowed characters and randomly picking an index in that string would do away with your goto's.

I think too a StringBuilder would do a lot better for storing the characters than a LinkedList.

I would use a HashSet:

HashSet<byte> validChars = new HashSet<byte> { 48, 49, 50 };  //add all valid

........

rng.GetBytes(buffer);
if(validChars.Contains(buffer[0]))
{
    chars.Add(char(buffer[0]));
    ++counter;
}


Guess user number between 0 and 100 (Binary Search) I am at the beginning of learning python and one of my homeworks (in chapter regarding loops) where to write a code that would guess user number between 1 - 100.

I believe I used algorithm called binary search.

# Introduction
print('\nThink about a number between 0 - 100 and let me guess which is it.',
      '\n\nI will ask you whether my guess is lower or higher than your number',
      '\nIndicate "l" if lower than your number',
        '"h" if higher than your number and',
        '"e" if my guess is correct.'
      '\n\nYou ready?, let"s start!\n')

# Algorithm
count = 0
lborder = 0
hborder = 100
guess = 50

while True:
    answer = input('Is your number higher, lower or equal to ' + str(guess) + ':')
    if answer == 'l':
        hborder = guess
        guess = round((lborder + guess)/2)
    elif answer == 'h':
        lborder = guess
        guess = round((hborder + guess) / 2)
    elif answer == 'e':
        break
    else:
        print('Unrecognizable answer. Use: "l", "h", "e"')
    count += 1

print('\nHooray! It took me', count, 'to guess your number.', sep=' ')
input('\nPress Enter to Exit.')

My question is basically whether there is a better algorithm for finding those numbers (quicker).
Also do you have any other comments about my code.

Your calculation of count is incorrect in two ways:
 1. You don't count your last guess;
 2. You count invalid user input.

I would separate the input validation from the input processing. And I would add some logic to handle user input that leads to trying the same guess repetitively, for this the user has to lie, but who guarantees us they won't?

You should use:

guess = int((hborder + guess) / 2)

instead of guess = round((hborder + guess) / 2) for correct computation.


Sum of squares of 2 largest numbers out of a list of 3 numbers The problem is:

  Define a procedure that takes 3 number arguments and returns the sum of the squares of the 2 largest numbers.

I am a CS graduate and I am revising my university notes. This is my code so far:

(define (square n) (* n n))

(define (square_of_larger_numbs a b c)
  (cond ((and (> a b) (> a c))
         (cond ((> b c)
         (+ (square a) (square b)))
               (else (+ (square a) (square c))))
        )(else (cond ((> b c)
                      (+ (square b) (square c)))
                      (else (+ (square b) (square c))))
               )
         )
  )

(display (square_of_larger_numbs 4 3 3))

It seems to be working but the code looks terrible unreadable. Please note that I am not allowed to sort yet, as the book I am reading hasn't mentioned sorting yet, so I assumed that sorting is not welcomed as a part of the solution.

How can I make this more efficient/ readable?

Specification

It is helpful to specify the solution before writing code. The mathematics of sorting three values (a b c) means there are six (3!) permutations:

(define abc (a b c))
(define acb (a c b))
(define bac (b a c))
(define bca (b c a))
(define cab (c a b))
(define cba (c b a))

The code must cover all six possibilities.

There are three possible outputs:

(define output-ab (+ (* a a) (* b b)))
(define output-ac (+ (* a a) (* c c)))
(define output-bc (+ (* b b) (* c c)))

The code must map the six permutations to the three outputs.

abc || bac -> output-ab
acb || cab -> output-ac
bca || cba -> output-bc

Now the code can reflect the specification

   (cond

     ((or (and (>= a b)
               (>= b c))
          (and (>= b a))
               (>= a c)))
      output-ab)

     ((or (and (>= a b)
               (>= c b))
          (and (>= a b))
               (>= c a)))
      output-ac)

     ((or (and (>= b a)
               (>= c a))
          (and (>= c b))
               (>= b a)))
      output-bc)

     (else (error "unhandled condition")))

Review

Style

Scheme has formatting conventions. The code does better with indentation than with parenthesis.

Logic

The lack of a specification and tests based upon it leads to an incorrect implementation and poorly organized code. There is a branch which returns the same value on both paths:

 (else (cond ((> b c)
                (+ (square b) (square c)))
              (else (+ (square b) (square c))))

Efficiency

Correct code is the first order objective. Writing clear code facilitates that because it makes debugging and testing and maintenance easier.

Speed of execution matters only when it matters. That is always after the code is correct and after there is an objective performance issue, not just a theoretical one. Getting a wrong answer fast may be worse than no answer at all if we mistakenly believe the wrong answer is correct.

Edit: Also take a look at Sum of squares of two largest of three numbers and SICP Exercise 1.3 request for comments for more.

First of all the code isn't entirely correct, (s-o-l-n 2 3 1) returns
10 (choosing 3 and 1), where it should return 13.

Then a few points on the style:

Don't put the ending parens on their own line, that's not idiomatic
and looks weird.  You can (should) also choose an editor which will
automatically balance parens to make your life much easier (and also
keeps track of indentation).
Use dashes instead of underscores or camel-case -
square_of_larger_numbs should be square-of-larger-numbers in line
with usual Scheme names (few abbreviations).

For cond I would choose one style, either putting the clauses to the
right of the cond, or below; in either case, the indentation for the
else case should match the other cases.  Since you only have two
cases, using if would probably be preferred though, as there is no
benefit to using cond with only one form in the clause body.

And you could switch the order of operations around, first calculating
the squares and then comparing them, that way you get rid of a lot of
square calls.  Edit:  If you have negative input that wouldn't work though as @benrudgers commented, the code is corrected accordingly.

In summary I would write the following instead:

(define (square n) (* n n))

(define (square-of-larger-numbers a b c)
  (let ((a2 (square a))
        (b2 (square b))
        (c2 (square c)))
    (cond
     ((and (>= a b) (>= a c))
      (cond
       ((> b c)
        (+ a2 b2))
       (else
        (+ a2 c2))))
     ((and (>= b a) (>= b c))
      (cond
       ((> a c)
        (+ b2 a2))
       (else
        (+ b2 c2))))
     (else
      (cond
       ((> a b)
        (+ c2 a2))
       (else
        (+ c2 b2)))))))

(display (square-of-larger-numbers 4 3 3))

That is not the most efficient though, albeit readable (one case for
each maximum value).  The following implements a limited sort instead,
so the process is then sort the input (till where you want it to be
sorted), then process the sorted values.

Edit: You noted that sorting wasn't yet mentioned, but at least the comparison you already had were implementing a sort-of sort already, so given that I think this is still applicable.  The links above have some other suggestions that would also get rid of all sorting.

Take a look at
Sorting networks for
more information on that.  In Lisp/Scheme it is also possible to
abstract that kind of computation into a macro (cf. Let Over Lambda chapter 7.6 for an implementation of that), so assuming we had one
already, something like (sorting-network 3) would then generate a
function like sort-3 below.

Also note that the use of cond/set! here isn't the best, ideally a
separate macro rotate! would be used to switch a number of variables
in-place (i.e. (if (> c a) (rotate! a c))) and the return value would
use (values a b c) instead to prevent additional memory allocation if
applicable in the implementation.

(define (square n) (* n n))

(define (sort-3 a b c)
  (cond
   ((> c a)
    (let ((a* a))
      (set! a c)
      (set! c a*))))
  (cond
   ((> c b)
    (let ((b* b))
      (set! b c)
      (set! c b*))))
  (cond
   ((> b a)
    (let ((a* a))
      (set! b a)
      (set! a b*))))
  (list a b c))

(define (square-of-larger-numbers a b c)
  (let ((result (sort-3 a b c)))
    (+ (square (car result)) (square (cadr result)))))

(display (square-of-larger-numbers 4 3 3))


Simplified if return suggestion from Android Studio I have this very simple method:

private boolean isTheParticipantTalking(ParticipantID participantId){
    if(mCurrentParticipant != null) {
        return participantId.equals(mCurrentParticipant.getId());
    }
    return false; // no one is talking
}

I receive a warning stating that I can simplify the code in the following manner:

private boolean isTheParticipantTalking(ParticipantID participantId){
    return mCurrentParticipant != null && participantId.equals(mCurrentParticipant.getId());
}

In your opinion, is this change desirable? I understand that we go from 4 lines of code to just 1, but I find my way more understandable in one blick.

I prefer your method, because it's a lot clearer. But I would turn the if around (first check the input, then the normal action):

private boolean isTheParticipantTalking(ParticipantID participantId){
    if(mCurrentParticipant == null) {
        return false; // no one is talking
    }
    return participantId.equals(mCurrentParticipant.getId());
}

Another question is if this is actually the best approach.

Is a non-existing participant talking? That question really doesn't make so much sense.

You could use Objects.requireNonNull which seems to have been written exactly for cases like this, or throw your own NullPointerException if you want to.

You must understand that return means true for compiler while it concider that this expression performs, so in that case it might be true only if mCurrentParticipant != null and participantId.equals (mCurrentParticipant.getId () is true. In other cases (when mCurrentParticipant == null or when participantId.equals (mCurrentParticipant.getId () is false) it returns false which contradicts the condition than any boolean expression expected to be true for compiler, so this Android Studio solution is more clear than another ones.


Generate a unique numeric identifier for each instance of a class I want to generate a unique ID for every instance of a given class.

The ID is of type unsigned long
When an instance is deleted, the ID is freed (can be reused)

Now, I've come up with the following implementation:

Class Foo
{
public:

  static unsigned std::set<unsigned long> s_usedID;

  static unsigned long generateID() // Generate a valid ID
  {
    static unsigned long id = 0;

     while (Foo::isIDUsed(id)) // If all ID are taken, create an infinite loop!
        ++id;

     return id;
  }

  static void addID(unsigned long id) // Add a given ID to the set of used ID
  {
    s_usedID.insert(id);
  }

  static void removeID(unsigned long id) // Remove a given ID from the set of used ID
  {
    s_usedID.erase(id);
  }

  static bool isIDUsed(unsigned long id)
  {
    return s_usedID.count(id) == 1 ? true:false;
  }

  explicit Foo() : m_id(Foo::generateID())
  {
    Foo::addID(m_id); // ID is now taken
  }

  virtual ~Foo()
  {
    Foo::removeID(m_id); // Free the ID
  }

private:

  unsigned long m_id;

};

Now for me, this logically works.

Is there a better way to implement this?
Are there any optimizations that I could make?

A few remarks:

The biggest obvious issue is the violation of encapsulation. All methods which allow you to modify the id set are public which means that outside code can change it and break the inherent assumption that each instance will have a unique id.

The only code which should be able to modify the id set are the constructor and destructor.

If you now say "ah, that won't matter it's just my code and I know not to do this" - programming is a skill and like any skill you get better at it through practice. Which typically means writing code. Therefore you should take any opportunity to practicing writing good code since that helps to form good habits. There is no excuse for being sloppy.
This:

return s_usedID.count(id) == 1 ? true:false

can be written more succinctly as:

return s_usedID.count(id) == 1;

The implementation is obviously not thread-safe. Just something to be aware of.
One way to improve it, is to just use a variable to store the current maximum id and return the next one higher if you need a new one. If an id gets freed up then you could just not worry about it.
If you really want to re-use returned id's then store them in a returnedIds set. The basic logic then becomes:

If returnedIds has any elements pop first element from set and return it
Else new id becomes current_max_id + 1

As I think it is impossible to have 2 instances of a class to be created in the very same nanosecond, I guess that unique identifiers can be generated using the chrono library:

 std::chrono::high_resolution_clock m_clock;
 long unique_ID = std::chrono::duration_cast<std::chrono::nanoseconds>(m_clock.now().time_since_epoch()).count();

You can then cast to unsigned long. You may also consider adding a small sleep time.


Python - Random Mandala I did a project in Python where I combined user-input, random, and the turtle module to make a sort of "Mandala Generator", or a program that can generate a simple design either randomly or by the specifications of users. Is there any way I can shorten my code without drastically changing it?

#custom mandala project Python
import turtle
import random
import time

#is the game in progress?
drawing = False

#define turtle
pen = turtle.Turtle()
pen.setheading(90)
pen.pensize(3)
pen.speed(12)

#colors empty list
colors = []

#define variables
LINE1, LINE2 = "~" * 36, "~" * 24

#define check input function
def get_input(response, values):
    #make sure the input is in the choices
    while True:
        value = input(response).strip().lower()
        if value in values:
            return value

#define main function
def main():
    print(LINE1 + "\nCustom Mandala Generator\n" + LINE1)
    name = input("What is your name? ")
    print("In this custom mandala generator, you get to choose the size and colors of your mandala, or have the computer generate a random one,", name, ".\n" + LINE2)
    time.sleep(1)
    ready = input("Are you ready or not? ")

    if ready[0].lower() == "y":
        answer = get_input("Would you like to use our random generator function or our custom function? {random/custom} ", {"random", "custom"})

        #universal variables
        size = random.uniform(1.5, 2.5)
        fd = 75 * size

        if answer == "random":
            colors.extend(("darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple"))
            #rt = right turn
            rt = random.uniform(100, 300)

        elif answer == "custom":
            print(LINE2 + "\nYou can choose any colors from this list for your Mandala: \nDarkred, red, yellow, darkgreen, green, lightgreen, darkblue, blue, purple")
            color1 = get_input("What is the first color? ", {"darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple"})
            color2 = get_input("What is the second color? ", {"darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple"})
            colors.extend((color1, color2))
            rt = int(input("What angle would you like your Mandala to turn at? "))

        #pre-draw sequence
        print(LINE2 + "\nInitializing...")
        print("Determining features... \nColor... \nSize...")
        time.sleep(3)
        print(LINE1 + "\nYour final result should be drawing as of this point!\nThanks for using the generator,", name, "!")

        for counter in range(50):
            selection = random.choice(colors)
            pen.color(selection)
            pen.forward(fd)
            pen.right(rt)

    else:
        print("Come and use the custom generator later!")

main()

I don't know why you would want to shorten your code. It is already just 73 lines long (including the final blank line). Apart from that, "short" does not automatically mean "good". Maybe concise would be a better fit here, which I would interpret as "the code does what it should do without clutter". IMHO your code is very concise and readable at the moment and that should not be sacrificed to gain a few lines/bytes. We're not at Code Golf here.

Nevertheless, there are a few things you can improve easily.

Documentation

Python's official Style Guide has recommendations on how to comment code and functions. Function docstrings should always be defined inside the function body and be surrounded by triple quotes """...""". Applying this to your code it would look something like

def get_input(response, values):
    """make sure the input is in the choices"""
    # your code here

This will also allow Python's built-in help(...) function and most IDEs to pick up the documentation.

Avoid globals

You should always strive to avoid global variables whenever possible. E.g. in your code colors is not used outside of main() and should therefore be defined there. drawing does not seem to be used anywhere, so you might be able to drop it completely. LINE1 and LINE2 are two valid uses for globals. My only recommendation here would be to put them on two separate lines. Oh, and strictly speaking your comment on them is "wrong" since they are not used as variables but as constants, which is also supported by the ALL_CAPS_NAME (the Style Guide also says something on that).

Variable names

There is this piece of code which I find kind of amusing:

#rt = right turn
rt = random.uniform(100, 300)

Why not just call the variable right_turn or right_turn_angle right away then? The same reasoning would apply to fd, but you didn't care to write the comment there and left it as an exercise to the person looking at your code to find out that it actually stands for forward. You could also rename size to mandala_scale or so and then drop that #universal variables altogether since it does not really help to understand the code better. At least at the moment you could even get rid of size completely since its only use is to be multiplied by 75 on the next line and it's never used after that.

Other aspects

There is a lot of repetition when it comes to the list of available colors. Including the print, you define them four times. If you ever decide to add, change or remove a color from that list, there would be four places that change would need to be applied to. I would recommend to define them in one place, and reuse the list wherever it is needed.

# ...

colors = []
all_colors = ("darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple")
if answer == "random":
    colors.extend(all_colors)
    right_turn = random.uniform(100, 300)

elif answer == "custom":
    all_colors_string = ", ".join(all_colors)[:-2] # -2 to trim of the trailing space and ,
    print(LINE2 + "\nYou can choose any colors from this list for your Mandala: \n" + all_colors_string)
    color1 = get_input("What is the first color? ", all_colors)
    color2 = get_input("What is the second color? ", all_colors)

# ...

In the transformation the set notation ({...}) got lost, but I think your application will not really suffer from the (minuscule) performance impact because a tuple is used instead of a set.

Another handy feature I would like to point out to you is string formatting. So what's that? Let's look at an example from your code:

print("[...] or have the computer generate a random one,", name, ".\n" + LINE2)

Here you are using Python's implicit behavior to print the elements of a tuple separated by whitespace1. That leads to the following output (minus the annotation of course):

[...] or have the computer generate a random one, Alex .
~~~~~~~~~~~~~~~~~~~~~~~~
                                       does look off ~^

Note the unwanted whitespace between the name and the dot?
But we can do better! Python has very powerful string formatting capabilities (doc), and since you have tagged your question as Python 3, you can likely use the new f-string syntax (helpful blog post comparing all the different ways, specification (PEP 498)).
Rewriting the code from above using an f-string would lead to:

print(f"[...] or have the computer generate a random one, {name}.\n" + LINE2)

So f"... {name}" basically generates a string containing the value of name right at the spot we the curly braces and the name of the variable where.2 For the sake of completeness that generates the following output:

[...] or have the computer generate a random one, Alex.
~~~~~~~~~~~~~~~~~~~~~~~~

As a final note, you could/should also add

if __name__ == "__main__":
    main()

(explanation) to highlight what part of the code is actually run if this file is used as Python script.

Bonus: by removing unnecessary globals and comments your script is actually even shorter more concise now, although that was not the main goal of what I was trying to tell you.

1 Depending on your Python experience you might wonder where there is a tuple in that line or you will nod silently. In the first case I would recommend reading this SO post on how Python deals with functions that have a variable number of arguments (often varargs for short), and then to have a look at how the print function is defined in the documentation.
2 In fact, f-strings are way more powerful than this, but you should refer to the resources linked earlier if you want to know more.

I prefer to have at least one print statement per line of output. Instead of:

print('line1\nline2\line3')

I would rather write:

print('line1')
print('line2')
print('line3')

That way, when there is a large block of output on the screen and you want to find it in the file, it's obvious where in the code it might be.

Going to sleep before doing the actual work is dishonest. There is no benefit of having sleep(3) in the code, it's just wasting the user's time.

From a technical viewpoint, asking for the name of the user is unnecessary.

The next question is unnecessary as well since the user, if they choose not to produce a mandala, can always quit the program at any time.

The question about the angle should suggest some useful numbers or at least a range, to help people who don't like math.

The following line is a bold lie:

print("Determining features... \nColor... \nSize...")

By using the word mandala, you made me expect a colorful image with curved lines, the lines painted in black and the areas between filled with colors. Just drawing some straight lines doesn't qualify as a mandala for me. That part is something you should invest some work, to create beautiful images.


Project Euler #3 in C# - largest prime factor I just started doing the Project Euler challenges and right now I'm on Challenge #3 - The largest prime factor of 600851475143.

I wrote some code and tried to optimize it based on some other Stack Overflow users' advice.

What I got is this code which does its job, but I think it takes longer than it should - calculating high numbers like 600851475143 takes forever!

Am I in the right way? How could I optimize it?

using System;

namespace ProjectEuler_LargestPrimeFactor
{
    class Program
    {
        static long numberToFactor = 600851475143;

        static void Main(string[] args)
        {
            Console.WriteLine(LargestPrimeFactorOf(numberToFactor) + " is the largest prime factor of " + numberToFactor);
            Console.ReadKey();
        }

        static long LargestPrimeFactorOf(long n)
        {
            long lastPrimeFactor = 0;

            for (long i = 2; i < n; i++)
            {
                if (isPrime(i) && n % i == 0)
                {
                    lastPrimeFactor = i;
                    Console.WriteLine(i + " is a prime factor of " + n);
                }
            }

            return lastPrimeFactor;
        }

        static bool isPrime(long n)
        {
            if (n == 2) return true;
            if ((n > 2 && n % 2 == 0) || n == 1) return false;

            for (long i = 2; i <= Math.Floor(Math.Sqrt(n)); ++i)
            {
                if (n % i == 0) return false;
            }

            return true;
        }
    }
}

Here's my solution to this problem

    static void Main(string[] args)
    {
        long num = 600851475143;
        int count = 3;
        Stopwatch sw = Stopwatch.StartNew();
        while (num > 1)
        {
            if (num%count == 0)
            {
                num /= count;
            }
            else
            {
                count += 2;
            }
        }
        sw.Stop();
        Console.WriteLine("The largest prime factor of the number {0} is {1} ", 600851475143, count);
        Console.WriteLine("Time to calculate in milliseconds : {0}", sw.ElapsedMilliseconds);
        Console.ReadKey();
    }

First of all most of the exercises you will find on Project Euler will require some math formula or concept in order to achieve the highest possible performance. It's not pure programming and thus not the best place to practice it in my opinion (ofc assuming you don't know all the formulas) you will probably learn more math than programming in the process which is a good thing if that's what you are looking for. Long story short look for formula before you go into the programming.

Back to your actual code :

Why is your code working slow ?
 There are few points here that I want to make first method calls are slower than just a bunch of code in a single method again some problems you will find later in Project Euler will be pretty easy to solve but hard to optimize so the usual C# code has everything separated in methods and classes because the performance loss is not that big in a normal project however here this might be your only bottleneck in some cases. You should always think in perspective what if you had even bigger number ? Would your method still work fast enough ? A working program doesn't mean that it's a good program.

Next you start thinking what can you improve and why exactly you are doing certain things. Do you really need to know every single number that is prime ?

In my solution there is simple prime factorization starting from the smallest prime we keep increasing the current number we have until we get a number that we can divide only by itself i.e the  largest prime factor. 100,000 iterations run for about 3000-3200 ms on my machine.

On the surface, everything looks alright to me, but that Console.WriteLine() in your for loop is going to add extra processing time since you are writing out to the screen quite a lot. Try commenting that line out to see if it improves the speed of your application.


Should I write my fadeTo function differently? fadeTo = function(obj, time, to, cChange ){
    if ( typeof( time ) == "undefined" )
        time = 1000;
    if ( typeof( to ) == "undefined" )
        to = 1;

    if ( time <= 0 )
        return true;

    objSt = obj.style;
    objOpac = parseFloat(objSt.opacity) + 0;

    if ( typeof( cChange ) == "undefined" )
    {
        cChange = (to - objOpac)/(time/10);
    }

    objSt.opacity = objOpac + cChange;

    setTimeout( function(){
            fadeTo( obj, time - 10, to, cChange);
        }, 10);
};

This code was written as a replacement for fadeIn and fadeOut. I made this for learning purposes and it works fine.

I'm just not sure if I could write it better, and if it could cause problems in the future. I feel that I should use two functions instead: one as an init function, and one to finish the process without asking all the extra ifs and simple variable settings.

Well written animation functions typically use a tweening algorithm that checks the system clock to see how much time is actually remaining in the originally specified time allotment and then adjusts the animation increment to put things back on time, even if the timer events weren't coming in exactly on time or the browser was super busy and you weren't getting all the cycles you might want or the host CPU just isn't very fast.  Done well, this can even allow you to adjust the step value for smoother animations on more capable computers and coarser steps on less capable computers.

As you have it, you have a hard coded step function in time of 1/10th the time and thus 1/10th the total opacity change.  On capable computers, you can run much, much smoother than this.

Here's an example:

function now() {
    return new Date().getTime();
}

function fadeTo(obj, time, to, doneFn){

    // sanitize optional arguments
    if ( typeof time === "undefined" )
        time = 1000;
    if ( typeof to === "undefined" )
        to = 1;

    if ( time <= 0 )
        return true;

    var objSt = obj.style,
        originalOpacity = parseFloat(getComputedStyle(obj, null).getPropertyValue("opacity")),
        deltaOpacity = to - originalOpacity,
        sanitizeOpacity,
        minT = 30,
        startTime = now();

    // based on whether we're going up or down, set our santize function
    if (deltaOpacity > 0) {
        sanitizeOpacity = function(x) {
            // not more than 1
            return Math.min(x, 1);
        }
    } else {
        sanitizeOpacity = function(x) {
            // not less than zero
            return Math.max(x, 0);
        }
    }

    function step() {
        // calculate how much time has elapsed since start
        // and then calculate what the opacity should be set
        // to for the animation to be perfectly on time
        var elapsed = now() - startTime, completion;

        // if all our time has elapsed, then just set to final opacity
        if (elapsed >= time) {
            objSt.opacity = to;
            // we're done, call the completion function
            // with the this pointer set to the object that was animating
            if (doneFn) {
                doneFn.apply(obj);
            }
        } else {
            // still some time remaining
            // calculate ideal opacity value to be "on time"
            completion = elapsed/time;
            objSt.opacity = sanitizeOpacity((deltaOpacity * completion) + originalOpacity);
            // schedule next step
            setTimeout(step, minT);
        }

    }
    // start the animation
    setTimeout(step, minT);
}

Working demo: http://jsfiddle.net/jfriend00/hG3Wj/

In other comments:

You MUST use var in front of variables that should be local variables.
You need to be getting the original opacity as the computedStyle, not just reading opacity directly because you need to include style sheet opacity, if nothing is set directly on the object.
If you want to know if an argument is truly undefined, you need to use === to test for it so there can be no automatic type conversions.
I find it more efficient to define an inner function that is called repeatedly because then all of our argument sanitization and what/not only has to happen once, not on each iteration and we can have state variables in the outer closure.

You can more succinctly express

if ( typeof( time ) == "undefined" )
        time = 1000;

as

time == null && (time = 1000);


Regex to parse URLs for their correctness according to RFC 3986 I recently came to write a regex to parse URLs. Now I wonder, did I miss something? Did I make a mistake or could I have written it cleaner? That's why I'm here.

In order to write the regex, I took this paper as a reference  (together with some Wikipedia articles about URIs/URLs).

//According to http://www.ietf.org/rfc/rfc3986.txt
private static final String URL_UNRESERVED_CHARS = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
private static final String URL_UNRESERVED_SPECIAL_CHARS = "-._~";
private static final String URL_UNRESERVED = URL_UNRESERVED_CHARS + URL_UNRESERVED_SPECIAL_CHARS;
private static final String URL_RESERVED_GEN_DELIMS = ":/?#[]@";
private static final String URL_RESERVED_SUB_DELIMS = "!$&'()*+,;=";
private static final String URL_CHAR_ENCODING_SIGN = "%";

public static final String URL_ALLOWED_CHARS = URL_UNRESERVED + URL_RESERVED_GEN_DELIMS + URL_RESERVED_SUB_DELIMS + URL_CHAR_ENCODING_SIGN;

private static final String REGEX_SCHEME = "[A-Za-z][A-Za-z0-9+.-]*:"; //Also called 'protocol'
private static final String REGEX_AUTHORATIVE_DECLARATION = "/{2}";
private static final String REGEX_USERINFO = "(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})+(?::(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})+)?@";
private static final String REGEX_HOST = "(?:[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\\.){1,126}[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?";
private static final String REGEX_PORT = ":[0-9]+";
private static final String REGEX_PATH = "/(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})*";
private static final String REGEX_QUERY = "\\?(?:[A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)(?:[&|;][A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)*";
//FRAGMENTs don't need to be parsed as they won't be sent to the server anyways

public static final String REGEX_URL = "(?:" + REGEX_SCHEME + REGEX_AUTHORATIVE_DECLARATION + ")?(?:" + REGEX_USERINFO + ")?" + REGEX_HOST + "(?:" + REGEX_PORT + ")?(?:" + REGEX_PATH + ")*(?:" + REGEX_QUERY + ")?";

The above code snippet is written in Java (which explains for the \\ as they need to be escaped in Strings in order to be taken as literals). I split the regex up into multiple parts in order to increase readability and maintainability (although it technically shouldn't require any).

That would be the regex in its true form as when encountered in the wild:

(?:[A-Za-z][A-Za-z0-9+.-]*:/{2})?(?:(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})+(?::(?:[A-Za-z0-9-._~]|[%][A-Fa-f0-9]{2})+)?@)?(?:[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.){1,126}[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::[0-9]+)?(?:/(?:[A-Za-z0-9-._~]|%[A-Fa-f0-9]{2})*)*(?:\?(?:[A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)(?:&|;[A-Za-z0-9-._~]+(?:=(?:[A-Za-z0-9-._~+]|%[A-Fa-f0-9]{2})+)?)*)?

TL;DR Regex above does validate a URL against the defined rules here. I couldn't break it yet, but is it breakable? Can some of it be rewritten in a cleaner fashion?

Holy regex Batman!

Without testing, you should be able to replace all occurrences of:

[0-9] with \d
[A-Za-z0-9] with [\w^_]
[A-Fa-f0-9] with \p{XDigit}
[A-Za-z0-9-._~] with [\w.~-]

and so forth. See documentation for Pattern.

As for your capturing groups I'm quite sure you can do some clever back referencing there but I will save my sanity and not try to parse the regexp. :)

(Some how the update is not showing in the above answer)
Update: This is failing for even

ldap://[2001:db8::7]/c=GB?objectClass?one

The example is from RFC 3986.

Even I need a foolproof regex to validate URI entered by users.


Marking a rectangular region in a NumPy array as an image mask I'm currently working on creating a mask for an image. I have initialized a two-dimensional numpy zeros array. I now want to replace the values of the mask corresponding to pixels following some conditions such as x1< x < x2 and y1 < y < y2 (where x and y are the coordinates of the pixels) to 1.

Is there an easier way to do it (maybe through slicing) without looping through the mask like below

clusterMask = np.zeros((h, w))
for x in range(h):
    for y in range(w):
        if x <= clusterH + 2 * S and x >= clusterH - 2*S and y <= clusterW + 2*S and y >= clusterW - 2*S:
            clusterMask[x][y] = 1

It turns out that Numpy has various nifty ways of indexing. I found that my question can be solved by

clusterMask[clusterH - 2*S : clusterH + 2*S, clusterW - 2*S : clusterW + 2*S] = 1

As given in one of the comments, this link contains all information regarding numpy indexing:
https://docs.scipy.org/doc/numpy/reference/arrays.indexing.html


Count the number of inversions using Binary Indexed Tree in an array Original Problem: HackerRank

I am trying to count the number of inversions using Binary Indexed Tree in an array. I've tried to optimise my code as much as I can. However, I still got TLE for the last 3 test cases. Any ideas that I can further optimise my code?

# Enter your code here. Read input from STDIN. Print output to STDOUT
#!/usr/bin/python

from copy import deepcopy

class BinaryIndexedTree(object):
    """Binary Indexed Tree
    """
    def __init__(self, n):
        self.n = n + 1
        self.bit = [0] * self.n

    def update(self, i, k):
        """Adds k to element with index i
        """
        while  i <= self.n - 1:
            self.bit[i] += k
            i = i + (i & -i)

    def count(self, i):
        """Returns the sum from index 1 to i
        """
        total = 0
        while i > 0:
            total += self.bit[i]
            i = i - (i & -i)
        return total

def binary_search(arr, target):
    """Binary Search
    """
    left, right = 0, len(arr) - 1
    while left <= right:
        mid = left + ((right - left) >> 1)
        if target == arr[mid]:
            return mid
        elif target < arr[mid]:
            right = mid - 1
        else:
            left = mid + 1

    # If not found, return -1.
    return -1

T = input()
for iterate in xrange(T):
    n = input()
    q = [ int( i ) for i in raw_input().strip().split() ]
    answer = 0
    # Write code to compute answer using x, a and answer

    # Build a Binary Indexed Tree.
    bit = BinaryIndexedTree(n)

    # Copy q and sort it.
    arr = sorted(deepcopy(q))

    # index array.
    index = map(lambda t: binary_search(arr, t) + 1, q)

    # Loop.
    for i in xrange(n - 1, -1, -1):
        answer += bit.count(index[i])
        bit.update(index[i] + 1, 1)
    print answer

def __init__(self, n):
        self.n = n + 1
        self.bit = [0] * self.n

bit has a well-known interpretation in programming which distracts from the intended interpretation here. IMO even something as generic as data would be better.

    def update(self, i, k):
        """Adds k to element with index i
        """
        while  i <= self.n - 1:
            self.bit[i] += k
            i = i + (i & -i)

The loop condition would be more familiar as while i < self.n.

I don't see any reason to avoid +=.

A reference to explain the data structure would be helpful, because otherwise this is quite mysterious.

        mid = left + ((right - left) >> 1)

The reason for this particular formulation of the midpoint is to avoid overflow in languages with fixed-width integer types. Since that's not an issue in Python, I'd favour the more straightforward mid = (left + right) >> 1. It might even be a tiny bit faster.

    q = [ int( i ) for i in raw_input().strip().split() ]

    index = map(lambda t: binary_search(arr, t) + 1, q)

Why the inconsistency? I think it would be more Pythonic to use comprehensions for both.

    # Copy q and sort it.
    arr = sorted(deepcopy(q))

    # index array.
    index = map(lambda t: binary_search(arr, t) + 1, q)

This seems like a rather heavyweight approach. Why not just

    index = sorted((val, idx) for idx, val in enumerate(q))

?

Although asymptotically I don't see any reason why this would be \$\omega(n \lg n)\$, it has a few \$\Theta(n \lg n)\$ stages. The standard algorithm for this problem, which is essentially merge sort, has the same asymptotic complexity but probably hides a much smaller constant behind the \$\Theta\$ / \$O\$.

I'm pretty sure you can replace

def count(self, i):
    """Returns the sum from index 1 to i
    """
    total = 0
    while i > 0:
        total += self.bit[i]
        i = i - (i & -i)
    return total

with

def count(self, i):
    return sum(self.bit[i] for i in range(1,i))

also,

mid = left + ((right - left) >> 1)

is the same as

mid = (right + left) >> 1


Generating random numbers from thread timing in C++ I have the following code blocks that generate apparently random numbers by using thread timing variations.

I am looking for a general code review, and a specific focus on whether the results are "true random numbers".

For computers with more than two CPUs

#include<chrono>
#include<iostream>
#include<thread>
using namespace std;

#define Mili 7
#define Base 7
#define Thrs 2

typedef unsigned char      Num;
typedef unsigned long long Out;

volatile Out out,tmp;

void inline thr(Num const num){
 while(true)
  out=out*Base+tmp,tmp=tmp*Base+num;
}

int main(){
 thread ths[Thrs];
 for(Num i=0;i<Thrs;++i)
  ths[i]=thread(thr,i);
 this_thread::sleep_for(chrono::milliseconds(Mili));
 while(true)
  cout.write((char*)&out,sizeof(out));
}

For computers with two CPUs

#include<iostream>
using namespace std;

#define Base 7
#define Iter 53

typedef unsigned char      Num;
typedef unsigned long long Out;

volatile Out out,tmp;

int main(){
 for(Num i=0;i<Iter;++i)
  #pragma omp parallel for schedule(runtime)
  for(Num num=0;num<Iter;++num)
   out=out*Base+tmp,tmp=tmp*Base+num;
 while(true){
  #pragma omp parallel for schedule(runtime)
  for(Num num=0;num<Iter;++num)
   out=out*Base+tmp,tmp=tmp*Base+num;
  cout.write((char*)&out,sizeof(out));
 }
}

For computers with one CPU:

#include<chrono>
#include<iostream>
#include<thread>
using namespace std;

#define Nano 1
#define Mili 7
#define Base 7
#define Thrs 2

typedef unsigned char      Num;
typedef unsigned long long Out;

volatile Out out,tmp;

void inline thr(Num const num){
 while(true)
  out=out*Base+tmp,tmp=tmp*Base+num;
}

int main(){
 thread ths[Thrs];
  for(Num i=0;i<Thrs;++i)
   ths[i]=thread(thr,i);
 this_thread::sleep_for(chrono::milliseconds(Mili));
 while(true)
  cout.write((char*)&out,sizeof(out)),
  this_thread::sleep_for(chrono::nanoseconds(Nano));
}

A true random number generator (TRNG) is a source of bits such that each bit has a probability of exactly 1/2 of being 1. Your code is obviously not a TRNG since its bias will heavily depend on the timing of operations.

A pseudo-random number generator (PRNG) is a deterministic algorithm that produces bits which satisfy statistical tests designed to verify randomness. Your code is obviously not deterministic so it is not a PRNG.

What you have here is a source of entropy: a bit stream which is not fully deterministic. You're measuring the timing of your program which depends on the synchronization between wall time and the CPU, so you're using jitter as a source of entropy.

CPU jitter may be a potential source of entropy. This paper by Stephan Müller analyzes CPU jitter on an x86 CPU. It has acceptable statistical properties if done right. However, there is a far cry from having acceptable statistical properties to being a good entropy source. The Linux kernel maintainers debated whether to include Müller's code as an entropy source, and concluded against it. They found two problems:

Having good statistical properties is good enough for some applications, but it is not good enough for security. For security, unpredictability is necessary. Even a knowledgeable adversary must not be able to reproduce the output. That includes, for example, an adversary who has the exact same hardware running the exact same program.
The CPU, in fine, does not contain much if any actual physical source of entropy. At the physical level, it's pretty much a deterministic system, albeit an extremely complex one which is difficult to analyze fully. It's unclear whether the physical uncertainties are enough to reach macroscopic levels.

To make things worse (much worse!) your implementation of CPU jitter measurement is deeply flawed. CPU jitter randomness is based on the lack of synchronization between the real-time clock (RTC) and the CPU's clock. But your code doesn't go very far in this direction: it spends most of its time doing sleep_for()! sleep_for() is based on interrupts driven by the RTC, and so are context switches. Confronting the RTC with itself is fully predictable, so the only source of unpredictability is the timing of the few computations that you make.

I haven't made any mathematical analysis of your computations (what you do with out). The only reason I can see to use multiplication here is to exercise different parts of the CPU to increase jitter. You should document this kind of design choices.

An obvious potential problem with your function is that it's prone to short cycles. The transformation (out, tmp) → (out*Base+tmp, tmp*Base+num) has cycles; if you fall into them you'll emit repetitions for a while until the scheduler hits. Just for fun, I ran your code and inspected the output visually, which is degree 0 of statistical testing (serious statistical testing uses tools that run for hours). The single-CPU version emitted sequences of zeroes () now and then… that's an obvious failure.

0000c9f0  5c 4d 68 41 e1 95 77 a6  17 2e bb 7a c6 37 d6 27
0000ca00  54 a4 7f f0 5f ee 5b fc  54 a4 7f f0 5f ee 5b fc
*
0000cbb0  a1 48 55 36 71 47 4b 75  4f a1 8d c1 60 fc c8 f0
0000cbc0  52 f5 22 43 e1 1a c0 4d  52 f5 22 43 e1 1a c0 4d
0000cbd0  52 f5 22 43 e1 1a c0 4d  74 47 ad 9f bd ee 5f 90
0000cbe0  74 47 ad 9f bd ee 5f 90  74 47 ad 9f bd ee 5f 90
*
0000cc40  74 47 ad 9f bd ee 5f 90  e3 2e c7 50 a8 84 5a 1c
0000cc50  fb ea 92 19 2f 6c 98 8e  0b 6a 7a 9a ac 59 87 03
0000cc60  73 ac 9f 43 2f 52 ab e1  2b d8 86 ab 7b 97 98 fc

(The first column is the offsets, then , all in hex. A * stands for a sequence of all-zero lines.)

On the programming side, you should initialize all variables. Leaving variables uninitialized doesn't increase entropy (they're predictable on a given platform), makes your program harder to analyze, and allows the compiler to optimize parts of your code away (for example, since you can't distinguish between a final value of out that was produced from a particular starting value, and a final value that was produced from a different starting value, the compiler is allowed to perform no computation until the first time the value of out is written to). Also note that operations like out=out*Base+tmp are not performed atomically, there can be a context switch between the time out is read and the time out is written to, in which case the work of the other thread(s) is be overwritten.

Note: sometimes “TRNG” is used to mean a source of entropy, which I discuss below. An actual RNG can be built on top of a source of entropy (preferably multiple independent sources) by seeding a PRNG with the source of entropy. By this very weak definition, you have a TRNG, but as we've seen one that's neither good nor new.

All in all, you seem to be trying to generate random numbers by flailing around in the dark. That doesn't work. Generating random numbers is very difficult. In order to be taken seriously, a random number generator needs to come with a detailed analysis, including rationales for each sources, collected statistical data, mathematical models, etc. Not a code dump. Showing that you did no research by demonstrating ignorance of common terminology doesn't help your credibility either.

These codes generate true random numbers because:

They implement a linear congruential generator, which generate pseudo-random numbers.
Unpredictable data races upgrades the pseudo-randomness into true-randomness.
CPU time jitter upgrades the pseudo-randomness into true-randomness.

The CPUs are physical sources of entropy. Due to their energetic inefficiency, when executing a single instruction, they produce entropy, which is finally exchanged to the environment through the heatsink. This entropy impacts on the results of time jitter and data races.

To predict a number it is required to know the complete current state of the whole system, what is impossible thanks to the uncertainty principle.

(Naming the version for more than two CPUs normal, for two CPUs openmp and for one CPU paused)

The normal and paused versions offer better results when they are executed with higher priority, low workloads or by shielded CPUs, because this avoids, that the numbers remain unchanged for large sequences.

The quality of them differ:

normal.cpp: Offers the best performance and generate true random numbers thanks to data races.
openmp.cpp: Offers a high-performance alternative for computers with two CPUs, which also generate true random numbers thanks to data races.
paused.cpp: Offers the best quality despite the lowest performance. This generate true random numbers thanks to data races when is executed by at least two CPUs and time jitter by one or more CPUs.

The numbers are like cats of Schrödinger's paradox. They are undefined until they are read.

References

CPU Time Jitter Based Non-Physical True Random Number Generator

Software Random Number Generation Based on Race Conditions


Ask the user a number between 2 and 20 Here is my attempt:

#include <stdio.h>

int main()
{
    int number = -1;
    printf("Enter a number in range [2,20]\n");
    scanf("%d", &number);
    while (number < 2 || 20 < number)
    {
        while (getchar() != '\n')
        {
        }
        printf("Try again\n");
        scanf("%d", &number);
    }
    printf("Your number was: %d", number);
    return 0;
}

But the solution from the professor is the following:

#include<stdio.h>

int main(){
    int n;
    while(scanf("%d",&n)==0 || n <2 || n >20){
        while ( getchar () != ’\n’);
        printf( "Wrong. Try again\n");
    }
    printf ( "%n\n",n);
    return 0;
}

Both approaches seem to work fine but is there a benefit to his method? Is it possible that scanf changes variable even though it couldn't read in a correct number. In that case checking the result of the scanf function would protect the program from accepting a wrong answer.

Prof's transcribed code has weaknesses

#include<stdio.h>

int main(){
    int n;
    while(scanf("%d",&n)==0 || n <2 || n >20){
        while ( getchar () != ’\n’);
        printf( "Wrong. Try again\n");
    }
    printf ( "%n\n",n);
    return 0;
}

Infinity loop

When input is closed after a scanf("%d",&n) == 0, getchar() can eventual return EOF before '\n' and thus while ( getchar () != ’\n’); never ends.

Smart quotes

’\n’ vs. '\n'.  Transcription problem or is that in the original?  In any case, the posted code was certainly not compiled.

Invalid specifier

"%n" does not print the int value.  It is undefined behavior as "%n" matches an int *.

    //printf ( "%n\n",n);
    printf ( "%d\n",n);

Requirement weakness

"Ask the user a number between 2 and 20" is a classic ambiguity.  Depending on perspective are 2 and 20 acceptable or not?  Better to provide more detail

"Ask the user a number, inclusively, between 2 and 20" or the like.

OP code has issues too.

#include <stdio.h>

int main()
{
    int number = -1;
    printf("Enter a number in range [2,20]\n");
    scanf("%d", &number);
    while (number < 2 || 20 < number)
    {
        while (getchar() != '\n')
        {
        }
        printf("Try again\n");
        scanf("%d", &number);
    }
    printf("Your number was: %d", number);
    return 0;
}

Failure to check input return value

scanf("%d", &number); may have failed due to non-numeric text input.  Check the result.

// scanf("%d", &number);
if (scanf("%d", &number) == 1) Success();
else Fail();

Infinite loop

OP's code suffers same potential infinite loop on end-of-file

// while (getchar() != '\n') { }
int ch;
while ((ch = getchar()) != '\n' && ch != EOF) { }

Also check scanf() return value.

Functional difference

With non-numeric input, Prof's code tries again.  OP's code, which does not evaluate the return value of scanf(), technically leads to trouble on rare input error.  On input error, the value of number is indeterminate. See comment

The 2 locations to take user input is simply twice the amount of code that needs checking.  Better to take input from 1 call.  Production code would have a get_int() to handle this.  Something like getint()

  but is there a benefit to his method?

OP's' code does have an informative prompt.

Else not much, both need work.  Perhaps

// untested code
#include<stdio.h>
#define MIN_N 2
#define MAX_N 20

int main(void) {
  int cnt;
  int n;
  printf("Enter a number in range [%d,%d]\n", MIN_N, MAX_N);
  while((cnt = scanf("%d",&n)) == 0 || ((cnt == 1) && (n < MIN_N || n > MAX_N))) {
    int ch;
    while ((ch = getchar ()) != '\n' && ch != EOF) {
      ;
    }
    printf( "Wrong input. Try again\n");
  }
  if (cnt == 1) printf ( "%d\n",n);
  return 0;
}

Better code would read a line into a string and process the input with strtol().

The one plus on the prof's code is that it has only one call to scanf().  Not repeating code when unneeded is good.

The minus on both implementations is the use of scanf() at all.  Personally, I would use fgets() and strtol(), and perhaps ensured that the user hasn't typed something like "3 or 4".


Helper utilities for easier benchmarking in C I have this tiny library for measuring the execution time in milliseconds and returning the duration plus the result:

execres.h

/******************
* File: execres.h *
******************/

#ifndef EXECRES_H
#define EXECRES_H

#include <sys/time.h>

typedef struct {
    unsigned long long  millis;
    void*               result;
} ExecResult, *PExecResult;

PExecResult execTime(void* (*code)(void*), void* param);

#endif  /* EXECRES_H */

execres.c

/******************
* File: execres.c *
******************/

#include "execres.h"
#include <stdlib.h>

PExecResult execTime(void* (*code)(void*), void* param)
{
    PExecResult result;
    struct timeval tva;
    struct timeval tvb;

    if (!code)
    {
        return NULL;
    }

    result = malloc(sizeof(ExecResult));

    if (!result)
    {
        return NULL;
    }

    gettimeofday(&tva, NULL);
    result->result = code(param);
    gettimeofday(&tvb, NULL);

    result->millis = (unsigned long long)
                     (tvb.tv_sec * 1000 + tvb.tv_usec / 1000 -
                      tva.tv_sec * 1000 - tva.tv_usec / 1000);
    return result;
}

main.c

/***************
* File: main.c *
***************/

#include <stdint.h>
#include <stdio.h>
#include "execres.h"

static uint64_t Fibonacci(uint64_t n)
{
    switch (n)
    {
        case 0:
            return 0;

        case 1:
            return 1;

        default:
            return Fibonacci(n - 1) + Fibonacci(n - 2);
    }
}

int main()
{
    PExecResult result;

    puts("Begin computation.");

    result = execTime((void* (*)(void*)) Fibonacci, (void*) 40);

    printf("Fibonacci took %u milliseconds and returned %llu.\n",
           (unsigned) result->millis,
           (uint64_t) result->result);

    return 0;
}

Nice bit of code!  Here are a few suggestions:

Initialize your variables when you declare them.  This helps shorten the code a bit and reduce possible bugs.
You need documenting comments within your code.  Especially since you are considering this as a library.
I would remove the NULL check on code in your execTime method, and document that a valid function pointer should be passed in.  I would expect the code to break if I tried to time NULL, not NULL to be returned back.  The standard library does this same thing with a few of their functions, like strlen().
Good job checking the return from malloc().
I would use the C11 standard function timespec_get() for your timing code.  It's a C standard (important!) function that has nanosecond precision.
If you want a better Fibonacci algorithm, here is an answer I wrote up a while ago on it.
Good job using puts() in place of printf() when you can.
You don't have to return 0 at the end of main(). The C standard  knows how frequently this is used, and lets us omit it.

  C99 & C11 §5.1.2.2(3)

  ...reaching the } that terminates the main() function returns a
  value of 0.

millis = sec * 1000 + usec / 1000

would mean milli = kilo + nano or E-3 = E3 + E-9
find the mistake ; )
edit: found it, blame my self. units are divided, not multiplied
(sec / E-3) = (sec / 1) * E3 + (sec / E-6) * E-3

also, the exponent-notation gives float values: E3 == 1000.0, so i must cast to integer
a more simple solution, less invasive to the to-be-timed code, giving microseconds:
#include <stdio.h>
#include <sys/time.h>

typedef unsigned int uint;

uint usec_diff(struct timeval t1) {
  struct timeval t2;
  gettimeofday(&t2, NULL);
  return
      (t2.tv_sec  - t1.tv_sec ) * (uint)1E6
    + (t2.tv_usec - t1.tv_usec);
}

int main() {
  struct timeval t1;
  uint dt;
  gettimeofday(&t1, NULL);

  // do some work
  for (uint i = 0; i < (uint)1E9; i++) {}

  dt = usec_diff(t1);
  printf("dt = %i usec", dt);
}

verification:
gcc -o time-diff time-diff.c && time ./time-diff
dt = 2789569 usec
real    0m2.791s