Compare 2 dynamic jagged arrays of doubles and arrays of doubles I want to be able to compare 2 jagged arrays of doubles for value equality. The arrays can have any number of dimensions. Inside any dimension could also be another array etc.

It is known that the values of any array dimension is a double, null or another array, so it should be able to compare any of these with one function:

double[]
double[][]
double[][][]
double[][double[]][double[][]]

public static bool ArraysEqual(dynamic a, dynamic b)
{
    // Same objects or both null
    if (ReferenceEquals(a, b)) return true;

    // Just one object is null
    if (a == null ^ b == null) return false;

    // Different number of elements
    if (a.Length != b.Length) return false;

    // Array is empty
    if (a.Length == 0) return true;

    // Test each item in the arrays
    for (int i = 0; i < a.Length; i++)
    {
        // Same object or both null
        if (ReferenceEquals(a[i], b[i])) continue;

        // Just one object is null
        if (a[i] == null ^ b[i] == null) return false;

        var aIsArray = a[i].GetType().IsArray;
        var bIsArray = b[i].GetType().IsArray;

        // Just one object is an array
        if (aIsArray ^ bIsArray) return false;

        // Both are arrays (no need to check b, since it was exclusive or'd prior)
        // Stop evaluating if the recursive call returned false;
        if (aIsArray)
        {
            if (!ArraysEqual(a[i], b[i])) return false;
        }
        else if (a[i] != b[i])
        {
            return false;
        }
    }

    return true;
}

Nice question, and I am inspired to answer it, even though I don't use C#. I know enough about the algorithmic side to see a better way.

First though, the multitude of 1-liner if-conditions are bad practice. You should consider using braces even for these values.

The second thing, is the bitwise-XOR ^ operator you are doing in two places... this is conceptually broken, and unnecessary. A simple logical OR || comparison will be fine, since the previous check would return true if both values were null.

if (a == null || b == null)
{
     return false;
}

Now, about your algorithm.

The recursive system you have is a good option, but, if you change the 'recurse' point of the recursion, you will be able to do things in a more concise way. At the moment you are doing a number of the tests twice.

Have a look at the following:

public static bool ArraysEqual(dynamic a, dynamic b)
{
    // Same objects or both null
    if (ReferenceEquals(a, b))
    {
        return true;
    }

    // Just one object is null
    if (a == null || b == null)
    {
        return false;
    }

    if (a.GetType().IsArray && b.GetType().IsArray)
    {
        if (a.Length == b.Length)
        {
            for (int i = 0; i < a.Length; i++)
            {
                if (!ArraysEqual(a[i], b[i]))
                {
                    return false;
                }
            }
            return true;
        }
        return false;
    }

    // fall back to simple value compare
    return a == b;
}

Notice how, by doing the above, there is only one instance of each type of check.


Are there side-effects to having a generic Ninject factory? Consider this class:

using Ninject;
using Ninject.Syntax;
using Ninject.Parameters;

namespace MyApp.Dependencies.Factories
{
    public abstract class FactoryBase<T> where T : class
    {
        private IResolutionRoot _resolutionRoot;
        protected FactoryBase(IResolutionRoot resolutionRoot)
        {
            _resolutionRoot = resolutionRoot;
        }

        protected T Create(params IParameter[] parameters)
        {
            return _resolutionRoot.Get<T>(parameters);
        }
    }
}

What this class really abstracts away, is the _resolutionRoot.Get<T> part, allowing for concrete factory classes to look something like this:

//using Ninject; // not needed
using Ninject.Syntax;
using Ninject.Parameters;

public interface IUnicornFactory
{
    Unicorn Create(string name, int hornLength, Color color);
}

public class UnicornFactory : FactoryBase<Unicorn>, IUnicornFactory
{
    public UnicornFactory(IResolutionRoot resolutionRoot)
        : base(resolutionRoot)
    { }

    public Unicorn Create(string name, int hornLength, Color color)
    {
        return Create(new ConstructorArgument("name", name),
                      new ConstructorArgument("hornLength", hornLength),
                      new ConstructorArgument("color", color));
    }
}

Is this overkill, or something that I'll thank myself for later, when I have dozens of factory classes? I find the base factory merely allows for synctactic sugar in the concrete factory implementations - are there any side-effects to this?

Line-by-line I had exactly the same factory code. It smells, isn't it?

The correct solution would be to use Ninject Factory extension. I am happy since.


Unbounded Sieve of Eratosthenes in Swift I've spent a while teaching myself Swift, and decided to take on the challenge of writing an unbounded Sieve of Eratosthenes to challenge myself. This is actually the first time I've written an unbounded sieve, as well as the first large Swift program I've written, so all comments are appreciated!

This was written using Swift 3 in Xcode 8 beta 3, so it should work on Swift 3.0 Preview 2 from swift.org, but I have not tested it.

public struct PrimeSequence: Sequence {
    private let stop: StoppingPoint

    /// Construct a sequence of primes up to a max.
    ///
    /// - note: This constructor is eager.
    ///
    /// - complexity: `O(log n)`
    ///
    public init(max: Int) {
        self.stop = .precompute([1] + sieveOfEratosthenes(n: max))
    }

    public init(terms: Int? = nil) {
        self.stop = .terms(terms)
    }

    public func makeIterator() -> AnyIterator<Int> {
        switch (stop) {
        case .precompute(let primes):
            return AnyIterator(primes.makeIterator())
        case .terms(let terms):
            var sieve = UnboundedSieve()
            var term = 0
            return AnyIterator {
                term += 1
                if let terms = terms, term > terms {
                    return nil
                }
                if term == 1 {
                    return 1
                } else {
                    return sieve.next()
                }
            }
        }
    }

    private enum StoppingPoint {
        case precompute([Int])
        case terms(Int?)
    }
}

///
/// Lazy, Unbounded, Sieve of Erathosthenes.
///
/// See https://www.cs.hmc.edu/~oneill/papers/Sieve-JFP.pdf
///
private struct UnboundedSieve: IteratorProtocol {

    var sieve: PriorityQueue<PrimeCounter>
    var counter = 2

    init() {
        sieve = PriorityQueue { $0.composite < $1.composite }
    }

    private struct PrimeCounter {
        let prime: Int
        var composite: Int
        func next() -> PrimeCounter {
            return PrimeCounter(prime: prime, composite: composite + prime)
        }
    }

    mutating func next() -> Int? {
        while let catcher = sieve.peek(), catcher.composite == counter {
            while let catcher = sieve.peek(), catcher.composite == counter {
                sieve.replaceTop(with: catcher.next())
            }
            counter += 1
        }
        defer {
            sieve.enqueue(PrimeCounter(prime: counter, composite: counter + counter))
            counter += 1
        }
        return counter
    }
}

///
/// Mutating Sieve of Erathosthenes.
///
/// Source: https://developer.apple.com/videos/play/wwdc2015/414/ @ 8:15
///
/// I would use the more "swifty" version @ 10:00, but this is the "true" sieve.
///
/// Complexity: `O(n log log n)`
///
private func sieveOfEratosthenes(n: Int) -> [Int] {
    var numbers = [Int](2..<n)
    for i in 0..<n-2 {
        let prime = numbers[i]
        guard prime > 0 else { continue }
        for multiple in stride(from: 2 * prime-2, to: n-2, by: prime) {
            numbers[multiple] = 0
        }
    }
    return numbers.filter { $0 > 0 }
}

I'm using a heap-based PriorityQueue, whose implementation doesn't matter, but here's the protocol pseudocode:

protocol PriorityQueue<T> {
    init(sort: (T, T) -> Bool)
    enqueue(_ element: T)
    dequeue() -> T?
    peek() -> T?
    replaceTop(with value: T)
}

For the purposes of this review, you can assume that it is accurate (but you can see it here if you want).

1 is not a prime number!

By definition, a prime number is an integer greater than one which has
no (positive) divisor other than 1 and itself.

Both the UnboundedSieve
iterator and the sieveOfEratosthenes function – correctly – produce 2 as the first prime. But then in PrimeSequence, the number 1
is inserted and I see no reason to do so.

From the description and naming of the max parameter in the

/// Construct a sequence of primes up to a max.
/// ...
public init(max: Int)

constructor I would assume that it produces a sequence of all primes
up to and including max, but that is not true: It produces all
primes below max because

private func sieveOfEratosthenes(n: Int) -> [Int]

returns all primes below n. That might be unexpected. I would
suggest to change the function so that the upper limit is included,
and give the parameters a more descriptive name:

/// Creates a sequence of primes up to (and including) `limit`.
public init(upTo limit: Int) {

I don't see the benefit of the terms parameter in

public init(terms: Int? = nil)

because  PrimeSequence(terms: numberOfTerms)
can be replaced by PrimeSequence().prefix(numberOfTerms),
using the prefix() method from the Swift standard library.
I would suggest to remove that parameter.

That leaves us with only two cases: creating a finite sequence
(using the sieve of Eratosthenes), or an infinite sequence
(using a priority queue), and the PrimeSequence definition
can be simplified to

public struct PrimeSequence: Sequence {
    private let iterator: AnyIterator<Int>

    /// Creates a sequence of primes up to (and including) `limit`.
    public init(upTo limit: Int) {
        self.iterator = AnyIterator(sieveOfEratosthenes(upTo: limit).makeIterator())
   }

    /// Creates an "infinite" sequence of prime numbers.
    public init() {
        self.iterator = AnyIterator(UnboundedSieve())
    }

    public func makeIterator() -> AnyIterator<Int> {
        return iterator
    }
}

struct UnboundedSieve looks like a clean implementation of the
priority queue-based algorithm from https://www.cs.hmc.edu/~oneill/papers/Sieve-JFP.pdf to me. However,

sieve.enqueue(PrimeCounter(prime: counter, composite: counter + counter))

can be improved to

sieve.enqueue(PrimeCounter(prime: counter, composite: counter * counter))

since all primes less than counter have already been found.
This reduced the execution time of my test (code below) from
2.26 sec to 1.17 sec.

A possible further improvement is to treat the only even prime 2
separately and then handle only odd numbers:

private struct UnboundedSieve: IteratorProtocol {

    var sieve: PriorityQueue<PrimeCounter>
    var counter = 2

    init() {
        sieve = PriorityQueue { $0.composite < $1.composite }
    }

    private struct PrimeCounter {
        let prime: Int
        var composite: Int
        func next() -> PrimeCounter {
            return PrimeCounter(prime: prime, composite: composite + 2 * prime)
        }
    }

    mutating func next() -> Int? {
        if counter == 2 {
            counter = 3
            return 2
        }

        while let catcher = sieve.peek(), catcher.composite == counter {
            while let catcher = sieve.peek(), catcher.composite == counter {
                sieve.replaceTop(with: catcher.next())
            }
            counter += 2
        }
        defer {
            sieve.enqueue(PrimeCounter(prime: counter, composite: counter * counter))
            counter += 2
        }
        return counter
    }
}

The reduced the execution time of the test code to 0.58 sec.

In sieveOfEratosthenes() the same improvement as mentioned above
applies: If a prime number is found then "crossing out multiples"
can start at the square of that prime:

private func sieveOfEratosthenes(upTo n: Int) -> [Int] {
    var numbers = [Int](2...n)
    for i in 0...n-2 {
        let prime = numbers[i]
        guard prime > 0 else { continue }
        for multiple in stride(from: prime * prime-2, through: n-2, by: prime) {
            numbers[multiple] = 0
        }
    }
    return numbers.filter { $0 > 0 }
}

The reduced the execution time for my test code from
0.37 sec to 0.28 sec.

The numbers array in this function holds redundant information,
memory can be saved by replacing it a by an array of booleans
which marks all numbers which have been found to be composite:

private func sieveOfEratosthenes(upTo n: Int) -> [Int] {
    var result = [Int]()
    var composite = [Bool](repeating: false, count: n + 1)
    for i in 2...n {
        if !composite[i] {
            result.append(i)
            for multiple in stride(from: i * i, through: n, by: i) {
                composite[multiple] = true
            }
        }
    }
    return result
}

In my test this also turned out to be faster: 0.13 sec.

You could also implement the Eratosthenes sieve as an iterator,
which makes it non-eager and removes the results array:

private struct EratosthenesIterator: IteratorProtocol {
    let n: Int
    var composite: [Bool]
    var current = 2

    init(upTo n: Int) {
        self.n = n
        self.composite = [Bool](repeating: false, count: n + 1)
    }

    mutating func next() -> Int? {
        while current <= self.n {
            if !composite[current] {
                let prime = current
                for multiple in stride(from: current * current, through: self.n, by: current) {
                    composite[multiple] = true
                }
                current += 1
                return prime
            }
            current += 1
        }
        return nil
    }
}

The corresponding init method in struct PrimeSequence then becomes

    /// Creates a sequence of primes up to (and including) `limit`.
    public init(upTo limit: Int) {
        self.iterator = AnyIterator(EratosthenesIterator(upTo: limit))
   }

The execution time for the test code is now 0.12 sec.

Benchmarking code for the infinite generator:

Calculate the sum of the first 10,000 primes:

let numPrimes = 10_000
do {
    let startTime = Date()
    let sum = PrimeSequence().prefix(numPrimes).reduce(0, combine: +)
    let endTime = Date()
    print(sum, endTime.timeIntervalSince(startTime), "sec")
}

Benchmarking code for the Eratosthenes generator:

Calculates the sum of all primes below 10,000,000:

let N = 10_000_000

do {
    let startTime = Date()
    let sum = PrimeSequence(upTo: N).reduce(0, combine: +)
    let endTime = Date()
    print(sum, endTime.timeIntervalSince(startTime))
}

The tests were done on a MacBook, with the program compiled in Release mode.


c# Checking if string is HtmlDecode I have this string that I am sometimes double encoding it because it is coming from two different fields. I have build this method that will do a string StringComparison. It seems complicated wanted to know if there is a better way to do something like this.

 class Program
    {
        static void Main(string[] args)
        {

            string str1 = "This string is already encode &lt;0.1";

            Console.WriteLine(HTTPEncodeString(str1));

            string str2 = "This string needs to be encoded <0.01";
            Console.WriteLine(HTTPEncodeString(str2));

        }

        public static string HTTPEncodeString(string source)
        {
            if (EncodeText(source))
            {
                return source;

            }
            else
            {
                return HttpUtility.HtmlEncode(source);
            }

        }

        public static bool EncodeText(string val)
        {
            string decodedText = HttpUtility.HtmlDecode(val);
            string encodedText = HttpUtility.HtmlEncode(decodedText);

            return encodedText.Equals(val, StringComparison.OrdinalIgnoreCase);

        }
    }

This isn't a language I know, but there appears to be a problem with the logic.

Suppose I have a simple statement:

  In XML, ampersand can be represented using '&amp;' or '&#38;'

Is that an already-encoded string?  The logic here says that it is, but we see that's not the case - the HTML version of that would be

  In XML, ampersand can be represented using '&amp;amp;' or '&amp;#38;'

If it's hard to keep track in code, a better option might be a small class to hold a HTML-encoded string, so that we don't confuse these with content strings (or vice-versa, if HTML-encoded is the default in your target environment).

But TBH, I think that good variable naming should be enough to track which strings are which.


count pairs of matching integers in a collection in Common Lisp This defines count-pairs which

takes a collection of integers
returns the number of pairs of integers that are equal to each other.

If the input collection has these integers ...
1 2 0 2 1 3 2

... then count-pairs returns 2 because

one pair can be made with the 1's
a second pair can be made with the 2's
The leftover unpaired integers do not affect the answer.

(defun make-pair (table i)
  (setf
   ;; increment pair count:
   (gethash 'pairs table) (1+ (gethash 'pairs table 0))
   ;; make first half of the new pair unavailable:
   (gethash i table) nil)
  table)

(defun keep-first-half-of-pair (table i)
  (setf (gethash i table) t)
  table)

(defun first-half-available-p (table i)
  (gethash i table))

(defun consider-one (table i)
  (if (first-half-available-p table i)
      (make-pair table i)
      (keep-first-half-of-pair table i)))

(defun count-pairs (integers)
  (let ((table (reduce #'consider-one
                       integers
                       :initial-value
                       (make-hash-table))))
    (gethash 'pairs table 0)))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; boilerplate for running as a hacker rank submission
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(read-line) ;; we don't need n

(defvar integers
  (read-from-string
   (concatenate 'string
                "(" (read-line) ")")))

(format t "~a~%" (count-pairs integers))

Some questions I have:

What changes would make this more idiomatic?
Is there an alternative for setf that returns the modified container instead of the multiple values returned by the storing form for the last altered place? (See how I really want the container as the result of make-pair and keep-first-half-available-p ...)
Should I stick with the -p naming convention for predicates? It seems ending with a ? is possible and looks more readable to me.
I guess Emacs knows if is a macro and is indenting it that way?
If there is an entirely different and better way to solve in Common Lisp, please do let me know.

Thanks in advance!

Stylistic consideration: a global variable is usually written with double "*" (earmuffs), so the global variable integers should be written as *integers*.
Now, for your questions.

I think your code is not particularly unidiomatic, just a little bit complex to read. What I find more unusual is your use of reduce to iterate over a list. In fact reduce is normally called with a binary function that takes pairs of elements of the same type, while you use it as a kind of iterator. In your case I would use a more classical iterator, like mapc, or, even better for me, loop. Another thing that I don't like is the use of the table to store also the global counter that will be used as result.

Summing all these considerations, I will rewrite your solution using
local helper functions, to reduce the complexity and simplify the
use of the parameters, and substitute the iterator while reducing by
one the number of auxiliary functions.
 (defun count-pairs (integers)
  (let ((pairs 0)
        (table (make-hash-table)))
    (flet ((make-pair (i)
             (incf pairs)
             (setf (gethash i table) nil))
           (keep-first-half-of-pair (i)
             (setf (gethash i table) t))
           (first-half-available-p (i)
             (gethash i table)))
      (loop for i in integers
        if (first-half-available-p i)
          do (make-pair i)
        else
          do (keep-first-half-of-pair i)))
    pairs))

If you need to return the table there is no other possibility. In the above solution this is not necessary, since table is not returned by the functions.

It's just a convention that you can follow or not. If you want to stick to Common Lisp common style, it is better to use the -p convention.

Yes.

Finally, for the fifth point, I would prefer a more concise solution, that counts all the elements, and only at the end counts all the pairs, in this way:
(defun count-pairs (integers)
  "returns the numbers the pairs of the list integers"
  (let ((table (make-hash-table)))
    (loop for i in integers
          do (incf (gethash i table 0)))
    (loop for v being the hash-value of table
          sum (truncate v 2))))


Is it possible to use switch case for more than one array.indexOf I have some set of if conditions for a particular array using JavaScript.

activity is the array containing 10-12 items. This array is converted to a multiple select dropdown. Depending upon the value selected, I need to hide and show divs. I also need to set values of another input choice as "NA".

Here's the code written for the onchange of that multiple select dropdown.

activity = ["strategy session","sessions","virtual",...,"Other"]

if (activity.indexOf("strategy session") != -1) {
    $("#FoPStrategySession").show();
}

if (activity.indexOf("sessions") != -1) {
    $("#acprojectname").show();
    if (supportmodel == "Level") {
        $(".accombohide").hide();
        $("[title='Test']").val("NA");
        $("[title='Test2']").val("NA");
    }
}

if (activity.indexOf("virtual") != -1) {
    if (supportmodel == "Level") {
        $(".lvl3_consult").hide();
        $("[title='Test']").val("NA");
        $("[title='Test2']").val("NA");
    }
}

if (activity.indexOf("Other") != -1) {
    $("#acactivityother").show();
}

Is there any other way to efficiently write this code using switch case or any other method?

You could for example put all actions in an object

var actions = {
  "session": function() { console.log("session"); },
  "virtual": function() { console.log("virtual"); },
};

and loop over its keys:

var activities = [
    "session",
    "virtual",
    "other"
];

for(let i = 0; i < activities.length; i++) {
  let action = actions[activities[i]];
  if (action) action();
}


Printing twin primes less than a given natural number n I have two programs doing this task correctly, but at first I'm not sure if the code is correct (I mean absence of not efficient and superfluous pieces of the code). Then, I can't choose which one is better and smarter. Couldn't you do it for me?

First:

int n, p, q;

scanf_s("%d", &n);

p = 1;
q = 2;

char flag = 1;

while (q < n)
{
    flag = 1;
    for (int i = 2; i <= sqrt(q); i++)
    {
        if (q%i == 0)
        {
            flag = 0;
            break;
        }
    }
    if (flag)
    {
        if (q - p == 2)
        printf("%d %d\n", p, q);
        p = q;
    }
    q++;
}

Second:

int n=0, p=3, q=5, f = 0;
    scanf("%d", &n);
    while (q < n)
    {
        f = 1;
        for (int i = 2; i <= p / 2; i++)
            if (p%i==0) { f = 0; break; }

        for (int i = 2; i <= q / 2; i++)
            if (q%i == 0) { f = 0; break; }

        if (f) printf("p = %d, q = %d\n", p, q);
        p++;
        q++;
    }

The differences between the two pieces of code are piling up.

The ones that I notice right away are the differences in the style or formatting

Let's first format them the same so that we can tell what is actually different.

This is the second piece of code, in which there are still some things I don't like, but we will get to that.

int n, p, q, f;
n = 0;
p = 3;
q = 5;
f = 0;

scanf("%d", &n);

while (q < n)
{
    f = 1;
    for (int i = 2; i <= p / 2; i++)
    {
        if (p%i==0)
        {
            f = 0; break;
        }
    }
    for (int i = 2; i <= q / 2; i++)
    {
        if (q%i == 0)
        {
            f = 0;
            break;
        }
    }
    if (f)
    {
        printf("p = %d, q = %d\n", p, q);
    }
    p++;
    q++;
}

Here you could actually create you f variable inside of the While loop instead of at the top.

Don't one-line if statements so you don't have to use brackets for the For loop. dangerous coding.  Just use brackets (curly braces).

What are all these variables? I don't know what they are or what they do.  Please use more descriptive variable names.

I don't know what the standard for C is, but in C# you wouldn't declare all your Variables in one line, so this looks bad to me.

Sometimes you put spaces in between operators and variables and sometimes you don't, please use spaces.

The first piece of code looked like it was formatted pretty well. The only thing that I did was to break up the variable declarations and space some things out a little bit.

I would still recommend that you create more memorable variable names though.

int n;
int p;
int q;

scanf_s("%d", &n);

p = 1;
q = 2;

char flag = 1;

while (q < n)
{
    flag = 1;
    for (int i = 2; i <= sqrt(q); i++)
    {
        if (q % i == 0)
        {
            flag = 0;
            break;
        }
    }
    if (flag)
    {
        if (q - p == 2)
        printf("%d %d\n", p, q);
        p = q;
    }
    q++;
}

Not assigning the variable n is probably a really bad idea, definitely not a good habit to fall into for sure.

I have been corrected, it is okay to not assign to n but you don't check the value returned by the scanf and that could result in a null reference exception

One thing that I noticed is that you are using either an integer or a char datatype for a flag, why wouldn't you use a boolean type or even a bit instead?

It looks like you could use an #include if you are in C99.

#include <stdbool.h>

but it does appear that the other options are integers except for one which is an Enum

Option 1

typedef int bool;
#define true 1
#define false 0

Option 2

typedef int bool;
enum { false, true };

Option 3

typedef enum { false, true } bool;

Reference: answer to -> Using boolean values in C

So I guess that it would make sense to use an integer.  The char might even make sense since it is an integer type.  But I think it would be less confusing if you defined a data type or at the very least label it flag and not f.


Error handling on temperature sensor I built an allsky camera with a temperature sensor and a heater. The heater is supposed to turn when the sensor (measure_temp()) return a temperature below 8, which is checked every 15 minutes. However, about every 4th measurement fails and so I included an error handler that returns 7, so that the heater turns on if the sensor returns an error, but it seems like a bad solution. Is there a better way to handle the error like run the function until no error occurs?
import RPi.GPIO as GPIO
import time
import datetime
import time
import board
import adafruit_dht

def measure_temp():
    dhtDevice = adafruit_dht.DHT22(board.D18)

    try:
        temperature = dhtDevice.temperature
        error_test = ""
    except RuntimeError as error:
        temperature = 7
    except:
        temperature = 7
    return temperature

if __name__ == '__main__':

    while True:
        if  measure_temp() < 8:
            GPIO.setmode(GPIO.BCM)
            RELAIS_1_GPIO = 17
            GPIO.setup(RELAIS_1_GPIO, GPIO.OUT)
            GPIO.output(RELAIS_1_GPIO, GPIO.HIGH)
            print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
            print('Heating on')
            print()
            time.sleep(900)
            GPIO.cleanup()
        else:
            GPIO.setmode(GPIO.BCM)
            RELAIS_1_GPIO = 17
            GPIO.setup(RELAIS_1_GPIO, GPIO.OUT)
            GPIO.output(RELAIS_1_GPIO, GPIO.LOW)
            print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
            print('Heating off')
            time.sleep(900)
            GPIO.cleanup()

Every 4th measurement fails is a pretty high error rate. The preferred course of action would be to root cause the error and fix it. Meanwhile, since the error seems to be transient, you may want to read the sensor again, and again, until it reads cleanly. Another strategy is to read more often, and maintain the running average (discarding the failures).
For a proper review, DRY. The GPIO setup shall be lifted out of the if/else:
while True:
    GPIO.setmode(GPIO.BCM)
    RELAIS_1_GPIO = 17
    GPIO.setup(RELAIS_1_GPIO, GPIO.OUT)
    if  measure_temp() < 8:
        GPIO.output(RELAIS_1_GPIO, GPIO.HIGH)
        print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
        print('Heating on')
        print()
    else:
        GPIO.output(RELAIS_1_GPIO, GPIO.LOW)
        print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
        print('Heating off')
    time.sleep(900)
    GPIO.cleanup()

I also recommend to factor the printing out to the function, e.g.
def log_heating_status(status):
    print('{:%d.%m.%Y %H:%M:%S}'.format(datetime.datetime.now()))
    print('Heating {%s}'.format(status))

to be called as log_heating_status('on') and log_heating_status('off').
It is also very unclear why do you setup/cleanup GPIO on every iteration. I'd expect it to be done once, outside the loop.


Optimizing "simplifier" Basically, I'm trying to make a Simplifier that can take inputs in the form of a String and print out the solution step-by-step. Now that I've got it working, I think it has some cleanliness issues.

public static void test(String eqn) {
    char[] order = { '^', '*', '/', '+'};   //order of precendece
    int found, i, j;
    eqn = eqn.replace("-", "+-");       //Subtracting = Adding (-)ve number.
    eqn = eqn.replace(" ", "");         //Spaces can cause a lot of trouble.
    String eqnC, left, right;
    System.out.println("=>\t"+eqn);
    for(char op : order) {
        while ((found = eqn.indexOf(op)) > -1) {
            left = eqn.substring(0, found);
            right = eqn.substring(found + 1);
            //keep 'walking' up and down the String till a non-number encoutered.
            for( i = left.length() - 1; i >= 0; i--)
                if(!isPONumber(left.charAt(i)))
                    break;
            for( j = 0; j < right.length(); j++)
                if(!isPONumber(right.charAt(j)))
                    break;
            double lV = Double.parseDouble(left.substring(++i));    //left operand
            double rV = Double.parseDouble(right.substring(0, j));  //right operand
            eqn = left.substring(0, i) + putV(lV, rV, op) + right.substring(j);
            System.out.println("=\t" + eqn);
        }
    }
}

private static String putV(double a, double b, char op) {
    switch(op) {
        case '^':   return Math.pow(a, b) + "";
        case '/':   return (a / b) + "";
        case '*':   return (a * b) + "";
        case '+':   return (a + b) + "";
    }
    return null;
}

/*
 * Limit (for now) : decimal support.
 */
private static boolean isPONumber(char c) {
    return Character.isDigit(c) || c == '.' || c == '-';
}

Here are some outputs:

=>  2*84/6+7^2
=   2*84/6+49.0
=   168.0/6+49.0
=   28.0+49.0
=   77.0

=>  3^3/3^2
=   27.0/3^2
=   27.0/9.0
=   3.0

=>  400^0.5
=   20.0

I'd be grateful if anyone would comment and criticize and help me to clean-up my code. I also invite suggestions for making it better, improvising it and so on.

Algorithm
The algorithm works well enough. You have captured the essence of the problem in your code, and reading your code it is apparent what it does, and why.
This is a good thing. It is not very often that the intent of the code is so easy to discern.
The draw-back of your algorithm is that it is not going to be able to support the more esoteric operations like parenthesis e.g. ( 123 - 45 ) ^ 2 .... That sort of support will require a preprocessing step.
Keeping the equation as a string is convenient for some things, but i would have preferred to see the process broken down in to components, for example, a parse step could break the input down to List<String> eqnParts = parse(eqn). at the same time, it could be trimming the white-space. Then, you just need to scan the List<String> and find 3 items separated by the highest precedence operator, and swap all three with the resulting value.
Code Style
You should read through the Code Style Guidelines for Java.
This code, for example, has a number of faults:

        for( i = left.length() - 1; i >= 0; i--)
            if(!isPONumber(left.charAt(i)))
                break;
        for( j = 0; j < right.length(); j++)
            if(!isPONumber(right.charAt(j)))
                break;

It should rather look something like:
        for (int i = left.length() - 1; i >= 0; i--) {
            if(!isPONumber(left.charAt(i))) {
                break;
            }
        }
        for (int j = 0; j < right.length(); j++) {
            if(!isPONumber(right.charAt(j))) {
                break;
            }
        }

Notes:

declare int i and int j inside the loop.
space after the keyword for.
braces even for '1-liner' conditional statements.

Double.parseDouble
Double.parseDouble() throws the unchecked exception NumberFormatException.
The exception thrown does not give great deals of information about why the number is not valid.
I always recommend wrapping Double.parseDouble (and Integer.parseInteger(), etc.) in a try-catch that reports what value was parsed, as well as the exception that was thrown. Otherwise debugging is a pain.
I often have a helper function like:
public static final double parseDouble(String value) {
    try {
        return Double.parseDouble(value);
    } catch (NumberFormatException nfe) {
        throw new IllegalArgumentException("Unable to Double.parseDouble(" + value + ")", nfe);
    }
}


Number-guessing game, for numbers between 0 and 99 I have written a complex number guess game. The output should be like this:

Enter initial guess between 0 and 99:
45
Initial guess is 45
45 is too high. Make a guess between 0 and 45
10
10 is too high. Make a guess between 0 and 10
50
50 out of range. Make a guess between 0 and 10
2
2 is too low. Make a guess between 2 and 10
8 is the correct guess! Game over!

What I wrote is this:
int main(){

srand(time(NULL));
int rannum;
int guess;
int max1=0,min2=0;
rannum = rand() % 99 + 1;

printf("Enter guess");
scanf("%d", &guess);

printf("%d", guess);

while (guess<99 && guess>0){
    if (rannum < guess){
        guess = max1;
        printf("%d is too high,0 and %d", max1, max1);
    }
    scanf("%d", &guess);

     if (guess > max1){

        printf("%d is out of range,enter between 0 and %d", guess, max1);
    }
     scanf("%d", &guess);
     if (guess < rannum){
        guess = min2;
        printf("%d is too low,guess between %d and %d", min2, max1, max1);
    }
     scanf("%d", &guess);
     if (guess == rannum){
         printf("You win");
     }
}

return 0;
}

It works somehow, but I would like some input on how well it's written.

This seems pretty straightforward and is readable. The variable names are pretty good. I'd change the following things.

Use Functions for Clarity

Speaking of functions, I recommend thinking about the problem you're solving as a set of steps, and then putting each step into its own function. In this case, there are several steps that need to occur:

Generate the number to be guessed
Prompt the user for their guess
Check their guess
If it was correct, tell them and exit
Otherwise, tell them what was wrong
Repeat steps 2-6

So you could write a function for most of those things:

int generateRandomNumber (int low, int high)
{
    int range = high - low + 1;
    int randomNum = low + rand() % range;
    return randomNum;
}

int promptUserForGuess(const char* prompt)
{
    printf(prompt);
    int guess = 0;
    scanf("%d", &guess);
    return guess;
}

typedef enum {
    ERR_NONE,
    ERR_TOO_LOW,
    ERR_TOO_HIGH,
    ERR_OUT_OF_RANGE
} GuessError;

GuessError checkGuess(int target, int low, int high, int guess)
{
    GuessError result = ERR_NONE;

    if ((guess < low) || (guess > high))
    {
        result = ERR_OUT_OF_RANGE;
    }
    else if (guess < target)
    {
        result = ERR_TOO_LOW;
    }
    else if (guess > target)
    {
        result = ERR_TO_HIGH;
    }

    return result;
}

void displayErrorOrSuccess(GuessError err, int low, int high, int guess)
{
    switch (err)
    {
        case ERR_NONE:
            printf ("You win\n");
        break;

        case ERR_TOO_LOW:
            printf ("%d is too low. Guess between %d and %d\n", guess, guess, high);
        break;

        case ERR_TOO_HIGH:
            printf ("%d is too high. Guess between %d and %d\n", guess, low, guess);
        break;

        case ERR_OUT_OF_RANGE:
            printf ("%d is out of range. Enter a guess between %d and %d\n", guess, low, high);
        break;
    }
}

Then your main() function becomes really simple and easy to read:

int main () {
    srand((unsigned)time((time_t*)NULL));
    int rannum = generateRandomNumber(1, 99);

    GuessError err = ERR_NONE;
    do {
        int guess = promptUserForGuess("Enter a guess:\n");
        err = checkGuess(rannum, 1, 99, guess);
        displayErrorOrSuccess(err, 1, 99, guess);
    } while (err != ERR_NONE);
}

Avoid Magic Numbers

I notice in your code that you have the value 99 written several times. What happens if you want to change the range of the random number? You'll have to update it each place it occurs in the code. We call bare numbers like these "magic numbers" because it's impossible to tell what they mean when reading the code. I recommend making some constants like this:

const int MIN_RANDOM_NUM = 1;
const int MAX_RANDOM_NUM = 99;

Then in your code you would use the named constants instead of the bare numbers. It immediately becomes obvious what they mean.

Bugs

There are a number of bugs in your original version. You're deciding whether to continue or not based on whether the guess is out of range or not. If the user enters a guess out of range, the program stops before it can tell the user what the problem is. You should be basing whether to stop or not on whether the user has guessed correctly. If the value is out of range, tell them that and have them enter a new guess.

Also, if they enter a value in range, it stops and waits for input a second time with no prompt. I think you meant for the first call to scanf() inside the while loop to be inside the first if. In fact, all of the calls to scanf() in the while loop should be inside the corresponding if statements.

You never set max1 and min2 to any useful values. So it always tells the user to enter a guess between 0 and 0. It would be best if they started out as max1 set to 99 and min2 set to 1, and then you updated them based on the value of the guess. For example if the user enters 5 and it's too low, the new minimum should be 5 for the next go round.


Leetcode: Cousins in Binary Tree Mu solution to Leetcode problem Cousins in Binary Tree works, but the code feels bulky. Is there a better way to solve this problem, particularly to use less additional variables? I will appreciate any suggestions on how to improve this code.
Problem:

In a binary tree, the root node is at depth 0, and children of each depth k node are at depth k+1.
Two nodes of a binary tree are cousins if they have the same depth, but have different parents.
We are given the root of a binary tree with unique values, and the values x and y of two different nodes in the tree.
Return true if and only if the nodes corresponding to the values x and y are cousins.

My code:
/**
* Definition for a binary tree node.
* struct TreeNode {
*     int val;
*     TreeNode *left;
*     TreeNode *right;
*     TreeNode() : val(0), left(nullptr), right(nullptr) {}
*     TreeNode(int x) : val(x), left(nullptr), right(nullptr) {}
*     TreeNode(int x, TreeNode *left, TreeNode *right) : val(x), left(left), right(right) {}
* };
*/

class Solution {
private:
    void is_cousins(TreeNode* root, int to_find, int depth,
        TreeNode* parent, pair<int, TreeNode*>& pair) {

    if (!root)
        return;

    is_cousins(root->left, to_find, depth + 1, root, pair);
    is_cousins(root->right, to_find, depth + 1, root, pair);

    if (root->val == to_find) {
        pair.first = depth;
        pair.second = parent;
        return;
    }
}

public:
    bool isCousins(TreeNode* root, int x, int y) {

        pair<int, TreeNode*> pairx = make_pair(0, nullptr);
        pair<int, TreeNode*> pairy = make_pair(0, nullptr);

        is_cousins(root, x, 0, nullptr, pairx);
        is_cousins(root, y, 0, nullptr, pairy);

        if (pairx.first == pairy.first && pairx.second != pairy.second)
            return true;
        return false;
    }
};

My first issue is that this function is badly named.

    void is_cousins(TreeNode* root, int to_find, int depth,
        TreeNode* parent, pair<int, TreeNode*>& pair)

It has nothing to do with cousins. It is simply finding the depth and parent of a particular value. You should name it appropriateely.

I don't like passing output parameters to a function:

    void is_cousins(TreeNode* root, int to_find, int depth,
        TreeNode* parent, pair<int, TreeNode*>& pair)

The parameter pair is the output of the function. Why not just return this value. That would be more logical and easier to read.

You don't have a truth state on whether the value x or y are found. I suppose you could say that nullptr in pair is a good way to test this. But that will make things complicated down the road when you try and use this function for other things that are not cousin related; so I don't like that idea. You could used a depth of -1 to indicate failure that would be better.

Talking of default values:

Your current default result is make_pair(0, nullptr) this means you found the root as the value (even if the root value does not equal left of right). This may work for your current situation out of sheer luck but you are returning a bad value.

i.e. A value that is not found in the tree and the root node will both return the same result.

Here is a major ineffeciency.

    is_cousins(root->left, to_find, depth + 1, root, pair);
    is_cousins(root->right, to_find, depth + 1, root, pair);

If you find the value on the left you still search the right subtree. This means you always search the whole tree even if you find the correct node very quickly.

You should check the result of the left search before doing the right search.

This is an anti patten

        if (pairx.first == pairy.first && pairx.second != pairy.second)
            return true;
        return false;

The pattern:

        if (test) {
            return true;
        }
        return false;

        // Can always be simplified to:

        return test;

This is a long handed way of writing:

         return pairx.first == pairy.first && pairx.second != pairy.second;

If you think that is too long for an expression to use in a reurn, then it is too long an expression for any context. In that case you should put the test in a named function so it is obvious what it is doing.

          return are_cousing_nodes_from_Depth(pairx, pairy);

     using DepthInfo = pair<int, TreeNode*>;
     inline bool are_cousing_nodes_from_Depth(DepthInfo const& lhs, DepthInfo const& rhs)
     {
         return lhs.first  == rhs.first    // Same depth
             && lhs.second != rhs.second;  // Not siblings
     }


Figuring out delivery times Is there anything that can be done to improve this code? Even if its using a comepltey different technique.

This code gets a list of vanruns with rules, cutoff time and fittinging time offset. We pass in the date (either now or for future that works out the best vanrun time) We want the latest cutofftime in that day for the same warehouse. Generally is this code OK for long term use or should it be broken down more? or possibly the entire logic is flawed? Use different mechanics? Just a review if anything seriously bad is happening.

private ServiceData.Models.CalculatedCutOff VarnRunEngine(List<Models.VanRunPrototype.VanRun> vanRuns, DateTime matchDate)
        {
            //get the date we are interested in
            System.DayOfWeek dayOfWeek = matchDate.DayOfWeek;
            DateTime dateOfWeekLoop = new DateTime(matchDate.Ticks);

            if (vanRuns == null | (vanRuns != null && vanRuns.Count == 0))
                throw new Exception("There are no Vanruns available to work out fitting times");

            Models.VanRunPrototype.VanRun vr = null;

            int loopkill = 0;
            while (true)
            {
                loopkill += 1;

                var _vrs = vanRuns.Where(v => v.DayOfWeek == (int)dateOfWeekLoop.DayOfWeek).ToList();
                var _vrss = _vrs.Where(v => v.CutOff > dateOfWeekLoop.TimeOfDay).ToList();
                var _vrsss = _vrss.OrderBy(t => t.CutOff).ToList();

                if (_vrsss != null && _vrsss.Count > 0)
                {

                    foreach (var v in _vrsss)
                    {
                        if (v.CutOff > DateTime.Now.TimeOfDay &&
                            (vr == null || (vr.FittingFor <= v.FittingFor && (v.CutOff > vr.CutOff && v.WareHouseID == vr.WareHouseID))))
                        {
                            vr = v;
                        }
                    }

                    vr = vr ?? _vrsss.First();
                    break;
                }

                //Go to next morning
                dateOfWeekLoop = dateOfWeekLoop.Date + new TimeSpan(8, 0, 0); //For 8 in da morning? or 00:00??
                dateOfWeekLoop = dateOfWeekLoop.AddDays(1);

                if (loopkill > 14)
                    throw new Exception("Infinite loop");

            }

            DateTime co = dateOfWeekLoop.Date + vr.CutOff;

            return new TyresAndServiceData.Models.CalculatedCutOff()
            {
                CutOffTime = co,
                FittingFor = co.AddMinutes(vr.FittingIn).AddMinutes(vr.FittingInAdjust ?? 0),
                QueryDate = matchDate,
                WareHouseID = vr.WareHouseID ?? 0
            };

        }

There are somethings that I want to point out, that you are doing slightly wrong.

The first one is the argument checking. Argument checking should always be the first thing you do in your code, unless it is an expensive operation or you can't do it in a straight forward manner. In your case you don't have those reasons, so move your argument checking to the beginning. You should also use ArgumentException when your arguments are invalid instead of Exception.
The second one is that you are not following the camelCase convetion for your variables, you are using _camelCase convention which applies to instance fields. Even if you are following your own convention which is not bad, I would not recommend a _camelCase convention.
You shouldn't call the method ToList from LINQ so often. Calling the method ToList transforms what may be a lazy collection to a eager collection (which occupy the memory).

Here follows the review I made to your code, I post some comments regarding some decisions I made. The code I provide may be further reviewed but I think this is a fair starting point.

ServiceData.Models.CalculatedCutOff VarnRunEngine(List<Models.VanRunPrototype.VanRun> vanRuns, DateTime matchDate)
{
    if (vanRuns == null || vanRuns.Count == 0)
        throw new ArgumentException("There are no Vanruns available to work out fitting times", vanRuns);

    System.DayOfWeek dayOfWeek = matchDate.DayOfWeek;
    DateTime dateOfWeekLoop = new DateTime(matchDate.Ticks);
    Models.VanRunPrototype.VanRun vr = null;
    int maxIterations = 14;
    for(int i = 0; i < maxIterations && vr == null; ++i)
    {
        var vrs = vanRuns.Where(v => v.DayOfWeek == (int)dateOfWeekLoop.DayOfWeek &&
                                     v.CutOff > dateOfWeekLoop.TimeOfDay)
                         .OrderBy(t => t.CutOff);

        //vrs will never be null, it may be empty though
        //But you don't really have to check count because you will be iterating it, meaning that there is no problem
        //if vrs is empty. But if you want to preserve this check for any reason you could use vrs.Any()
        //if (vrs != null && vrs.Count() > 0)
        //{
            foreach (var v in vrs)
            {
                if ((vr == null || (vr.FittingFor <= v.FittingFor &&
                                    v.CutOff > vr.CutOff &&
                                    v.WareHouseID == vr.WareHouseID)))

                {
                    vr = v;
                }
            }

            vr = vr ?? vrs.First();
        //}

        //Go to next morning
        dateOfWeekLoop = dateOfWeekLoop.Date + new TimeSpan(8, 0, 0);
        dateOfWeekLoop = dateOfWeekLoop.AddDays(1);
    }

    if(vr == null){
        throw new Exception(string.Format("Couldn't calculate CutOff in {0} iterations", maxIterations));
    }

    DateTime co = dateOfWeekLoop.Date + vr.CutOff;
    return new TyresAndServiceData.Models.CalculatedCutOff()
    {
        CutOffTime = co,
        FittingFor = co.AddMinutes(vr.FittingIn).AddMinutes(vr.FittingInAdjust ?? 0),
        QueryDate = matchDate,
        WareHouseID = vr.WareHouseID ?? 0
    };

}


Calculate square of a number without multiplication I wrote these two methods to calculate the square of a number:

static int sqr(int val)
{
    int valSqr = 0;

    for (int i = 0; i < val; i++)
        valSqr += val;

    return valSqr;
}

static int sqr(int val)
{
    int valSqr = 0;

    for (int i = 0, j = 1; i < val; i++, j += 2)
        valSqr += j;

    return valSqr;
}

The first one simply adds the given number to a value until it has been added number times.  The second one uses the 1+3+5... sequence.  I tried to profile this on Visual Studio, but the report was taking a really long time (over two hours) to be generated after the data collection was done.  I am particularly interested in which of these methods will run faster, and which is less resource intensive (would the second one be slightly more resource intensive because there are more variables?).

A more efficient way to do this is to keep doubling the product while you can....

So, for example, \$5^2\$ is:

$$\begin{eqnarray*}
 5 +&  5 &\longrightarrow 10 \\
 10 +& 10 &\longrightarrow 20
\end{eqnarray*} $$

then add the last 5 to get 25.

This can be done efficiently by using bit shifting:

public static int sqr (int val)
{
    int mask = 1;
    int factorsum = val;
    int sum = 0;
    while (val >= mask)
    {
        if ((val & mask) == mask)
        {
            sum += factorsum;
        }
        factorsum += factorsum;
        mask += mask;
    }
    return sum;
}

This is a \$O(\log(n))\$ solution to the problem. See it in ideone too


Age converter for some planets I was wondering if it is better to use:

    double onJupiter() {
     return AgeInSeconds / (jupiterOrbitalInEarthYears * ageInEarthSeconds);
    }

    double onMars() {
     return inputAgeInSeconds / (marsOrbitalInEarthYears * ageInEarthSeconds);
 }

Or:

  double onJupiter() {
        return onEarth() / JUPITER_ORBIT_RATIO;
    }

  double onMars() {
    return onEarth() / MARS_ORBIT_RATIO;
  }

In the second option, notice I called the onEarth() method twice. Does calling onEarth() create a brand new Stack (a bit like in recursion), hence it is not good to use? Because if I have a bunch of methods like onMars(), onMercury etc and they all call onEarth(), then would I end up creating a lot of new stacks?

Here are the Java classes in question:

Class 1:

class SpaceAge {

    private final double AgeInSeconds;
    private static final double ageInEarthSeconds = 31557600;
    private static final double marsOrbitalInEarthYears = 0.2408467;
    private static final double jupiterOrbitalInEarthYears = 11.862615;

    SpaceAge(double seconds) {
        this.AgeInSeconds = seconds;
    }

    double getSeconds() {
        return AgeInSeconds;
    }

    double onEarth() {
        return AgeInSeconds / ageInEarthSeconds;
    }

    double onJupiter() {
        return AgeInSeconds / (jupiterOrbitalInEarthYears * ageInEarthSeconds);
    }

    double onMars() {
        return AgeInSeconds / (marsOrbital * ageInEarthSeconds);
    }

}

Class 2:

class SpaceAge {

    private static final double EARTH_YR_IN_SECS = 31557600;
    private static final double MARS_ORBIT_RATIO = 1.8808158;
    private static final double JUPITER_ORBIT_RATIO = 11.862615;

    private final double ageInSeconds;

    SpaceAge(final double seconds) {
        this.ageInSeconds = seconds;
    }

    double getSeconds() {
        return this.ageInSeconds;
    }

    double onEarth() {
        return this.ageInSeconds / EARTH_YR_IN_SECS;
    }

    double onJupiter() {
        return onEarth() / JUPITER_ORBIT_RATIO;
    }

    double onMars() {
        return onEarth() / MARS_ORBIT_RATIO;
    }
}

Don't care about that. Unless you can prove that it creates a bottleneck, just stick with whatever seems the most readable and maintainable option.
 prefer the latter.

I'd also rename the methods. onEarth is too generic. What exactly on onEarth? Weeks? Months? Years? If it's years, call it getYearsOnEarth or something like this.


Inserting and updating record using LINQ by calling a common function I have a function named InserUpdateRecords(int flag) to which I pass integer variable which indicates the type of operation it has to perform , if flag==1 then insert , if flag==2 then update. Here is my code:

public void InsertUpdateRecords(int flag)
{
    Data_Customer_Log customer_log=new Data_Customer_Log();
    try
    {
        if(flag==2)
        {
            customer_log = (from cust in dt.Data_Customer_Logs
                            where cust.cApplicationNo == Request.QueryString["ApplicationNO"]
                            select cust).SingleOrDefault();
        }
        customer_log.cCustomerType = NewCustomerddlCustomerType.Text;
        customer_log.cBranchName = NewCustomerddlBranchName.Text;
        customer_log.nBranchNo = Convert.ToDecimal(Global.gBranchNo);
                     .
                     .
                     .
                     .
    }
    catch{}
}

So when I create Data_Customer_Log customer_log=new Data_Customer_Log();
and then depending on the value of flag I use it , is it an appropriate approach, what are the possible downsides to my approach.

I would probably pass in the customer log as a parameter to the method and then have it deal with just the database insertion.  If you wanted to keep a InsertOrUpdate type of approach I would branch on perhaps something like the object ID if it has one.  This way you can have a variety of ways that your object is created and the method is only responsible for the persistance of that object not also it's creation and populating.

A couple of problems I see in your approach:

The method is doing too many things.  It's responsible for querying for your record, instantiating the record and populating as well as performing the persistance.
The method has too many dependencies.  It looks like you have made it subject to both an external UI component in the .Text objects as well as the database layer.  What happens if you had multiple ways of populating the object.  You would have to duplicate this method?

Something like this (without knowing a bit more of your setup):

// a method higher up the chain
void CreateCustomerLog()
{
   Data_Customer_Log record = new Data_Customer_Log();
   record.cCustomerType = NewCustomerddlCustomerType.Text;
   record.cBranchName = NewCustomerddlBranchName.Text;
   record.nBranchNo = Convert.ToDecimal(Global.gBranchNo);

   InsertOrUpdate(record);
}

void UpdateCustomerLog(string applicationNo)
{
   Data_Customer_Log record = GetCustomerLog(applicationNo);

   if(record == null)
   {
      // handle record not existing here or filter it up the chain
   }
   else
   {
      InsertOrUpdate(record);
   }
}

Data_Customer_Log GetCustomerLog(string applicationNo)
{
   return (from cust in
           dt.Data_Customer_Logs
           where cust.cApplicationNo.Equals(applicationNo)
           select cust).SingleOrDefault();
}

Data_Customer_Log InsertOrUpdate(Data_Customer_Log record)
{
    if(record.ID > 0)
    {
       return Insert(record);
    }
    else
    {
       return Update(record);
    }
}

Data_Customer_Log Insert(Data_Customer_Log record)
{
    // TODO: perform insertion and throw exception if there is a problem
}

Data_Customer_Log Update(Data_Customer_Log record)
{
    // TODO: perform update and throw exception if there is a problem
}

I would probably point out that these methods would probably be in different classes.  For example the Update, Insert and/or Get might be in some sort of repository class.  The CreateCustomerLog and UpdateCustomerLog might be at the higher level, maybe your UI level as it interacts directly with the UI elements.

Just my thoughts.


A ViewModel using ReactiveUI 6 that loads and sends data Most of the Rx Compelling Examples™ load and store data locally, which I don't find that Compelling.

I spent some time coming up with how to best perform the following actions but want to make sure I am not Doing It Wrong™.

Here are the cases I am trying to solve:

Load data from cache and remote server when view model is instantiated
Save data to server when UI interaction happens (some button press)
Handle error cases (network down)

Some items I am trying to avoid:

Errors being swallowed (async in ctor for example)
Incorrect use of Rx patterns (is there a more 'correct' way)
Loss of data either through out of sync lists, not using cache correct, other.

public class CommonViewModel : ReactiveObject, IRoutableViewModel
{
    private readonly RemoteHttpService _remoteClient;

    private ReactiveList<Item> _items;
    public ReactiveList<Item> ItemsFromServer
    {
        get { return _items; }
        set { this.RaiseAndSetIfChanged(ref _items, value); }
    }

    public ReactiveCommand<List<Item>> LoadItems{ get; protected set; }
    public ReactiveCommand<object> SendItemsToServerCommand { get; protected set; }

    public string UrlPathSegment
    {
       get { return "CommonViewModel View"; }
    }

    public IScreen HostScreen { get; private set; }

    public CommonViewModel(IScreen screen)
    {
        HostScreen = screen;
        ItemsFromServer = new ReactiveList<Item>();
        _remoteClient = new RemoteHttpService(); //using refit and polly behind the scenes.

        SendItemsToServerCommand = ReactiveCommand.CreateAsyncTask((_, ctx) => InternalSendItemsToServer());
        SendItemsToServerCommand.ThrownExceptions.Subscribe(ex => UserError.Throw("Could not send data to server", ex));

        LoadItems = ReactiveCommand.CreateAsyncObservable(_ =>     this.InternalGetItemsFromCacheAndServer());
        LoadItems.Subscribe(list =>
        {
             _items.Clear();
            foreach (var item in list){
                 //using AddRange throw Presentation Exception
                _items.Add(item);
            }
         });
        LoadItems.ThrownExceptions.Subscribe(ex => UserError.Throw("Could not get data from server"));
        LoadItems.ExecuteAsyncTask();
    }

    private IObservable<List<Item>> InternalGetItemsFromCacheAndServer()
    {
        return BlobCache.LocalMachine.GetAndFetchLatest("items",
        InternalGetRemoteItems,
        dt => true,
        RxApp.MainThreadScheduler.Now + TimeSpan.FromMinutes(5));
    }

    private async Task<List<Item>> InternalGetRemoteItems()
    {
        var conferences = await _remoteClient.GetItems().ConfigureAwait(false);
        return conferences;
    }

    private async Task<object> InternalSendItemsToServer()
    {
        var itemsToSendToServer = ItemsFromServer.SomeLinq().Maybe();
        return await _remoteClient.SendDataToServer(itemsToSendToServer);
    }
}

Sorry to disappoint, but you did everything right! This is a quite good example of using RxUI, Akavache, and Refit together.

The only thing I would change, is to not immediately call LoadItems.ExecuteAsyncTask in the ViewModel constructor. Invoking this in the VM constructor means that your VM class becomes more difficult to test, because you always have to mock out the effects of calling LoadItems, even if the thing you are testing is unrelated.

Instead, I always call these commands in the View constructor, something like:

this.WhenAnyValue(x => x.ViewModel.LoadItems)
    .SelectMany(x => x.ExecuteAsync())
    .Subscribe();

This means that any time we get a new ViewModel, we execute LoadItems, which is what we want when the app is running, but not what we want in every unit test.


dlinfo(... SERPATH ...) C++ wrapper dlfcn.h is a library of C functions to work with dynamic dependencies (shared objects). I've been experimenting with wrapping them in a C++ API, mostly as practice in making a C++ wrapper.
This is an excerpt of my process of wrapping the Dl_serinfo parts of dlinfo(...) in the C library. For reference:

typedef struct {
    size_t dls_size;           /* Size in bytes of
                                the whole buffer */
    unsigned int dls_cnt;      /* Number of elements
                                in 'dls_serpath' */
    Dl_serpath dls_serpath[1]; /* Actually longer,
                                'dls_cnt' elements */
} Dl_serinfo;

And Dl_serpath:

typedef struct {
    char *dls_name;            /* Name of library search
                                path directory */
    unsigned int dls_flags;    /* Indicates where this
                                directory came from */
} Dl_serpath;

Note that the dls_serpath member of Dl_serinfo is a "flexible array member" and the entire Dl_serinfo struct is supposed to be allocated with enough space for dls_cnt entries in the array.

My wrapper:
#include <dlfcn.h>

#include <cstddef>
#include <memory>
#include <stdexcept>
#include <string_view>

namespace dl {
class dl_error : public std::runtime_error {
public:
    using std::runtime_error::runtime_error;
};

class serinfo;
class serpath {
    friend class serinfo;

private:
    std::string_view name_;
    unsigned int flags_;

    explicit serpath(::Dl_serpath path)
        : name_(path.dls_name)
        , flags_(path.dls_flags)
    {}

public:
    std::string_view name() const { return name_; }
    unsigned int flags() const { return flags_; }
};

class serinfo {
private:
    std::unique_ptr<std::byte[]> bytes;

public:
    explicit serinfo(void* dlhandle) {
        ::Dl_serinfo size;

        if (::dlinfo(dlhandle, RTLD_DI_SERINFOSIZE, &size)) {
            throw dl_error(::dlerror());
        }

        bytes = std::make_unique<std::byte[]>(size.dls_size);
        if (::dlinfo(dlhandle, RTLD_DI_SERINFOSIZE, bytes.get())) {
            throw dl_error(::dlerror());
        }

        if (::dlinfo(dlhandle, RTLD_DI_SERINFO, bytes.get())) {
            throw dl_error(::dlerror());
        }
    }

    unsigned int cnt() const {
        return reinterpret_cast<::Dl_serinfo const*>(bytes.get())->dls_cnt;
    }

    dl::serpath serpath(unsigned int index) const {
        return dl::serpath(
            reinterpret_cast<::Dl_serinfo const*>(bytes.get())->dls_serpath[index]);
    }
};
}

class dl_error : public std::runtime_error {
public:
    using std::runtime_error::runtime_error;
};

Given the purpose of this class, it seems logical to have it get the most recent error automatically. Especially since there is a gotcha involved, where dlerror() will return NULL if called a second time (so if some coder checks dlerror() to print or log the error message, and then does dl_error{::dlerror()}… boom). Plus it just makes it so much easier to use: throw dl_error{}; is just so much more ergonomic than throw dl_error{::dlerror()};.
Also, given that this class is already in namespace dl, it seems superfluous to have a prefix. dl::error is much nicer than dl::dl_error.
class serpath

Eh, I mean, is it really that much more painful to type out search_path (or search_path_info, given that it’s actually a path and some (in theory) flags)?
If you’re doing a thin wrapper around a C library, then it’s okay to recreate the C library’s shitty names. But that doesn’t look like what you’re doing, so why not use human-readable names? For someone whose first language isn’t English, the following is damn-near illegible:
auto const paths = dl::serinfo{dlhandle};

auto const path = paths.serpath(paths.cnt() - 1);

Compare that to:
auto const paths = dl::get_search_paths_for(dlhandle);

auto const path = paths[paths.size() - 1];

Every semi-competent C++ programmer will understand what size() and operator[] mean, and the function name is pretty much self-documenting. Meanwhile, ser isn’t even short for search (“serch” path?), and cnt just sounds rude.
Coders shouldn’t need to keep a translation guide open at hand to be able to read code, stopping every two lines to translate dl_serinfopathxyzagiuasgtha to something human-legible. 90% of the verbiage in good C++ code disappears into auto and the like anyway (and the fact that you need way fewer function calls than in C code), so take advantage of that and spell things out clearly.
class serpath {
    // ... [snip] ...

    std::string_view name_;

This is extremely unwise. I get that you’re trying to avoid unnecessary allocations, and you assume that the string name_ references will be owned by a serinfo object which will outlive the serpath object. But there’s nothing in the interface of either class that suggests that, let alone requires it. What’s to stop someone from doing:
auto get_particular_library_search_path(void* dlhandle)
{
    auto const info = dl::serinfo{dlhandle};

    for (auto i = 0u; i < info.cnt(); ++i)
    {
        auto const sp = info.serpath(i);

        if (satisfies_some_condition(sp))
            return sp;
    }

    throw std::runtime_error{"not found"};
}

If you’re going to keep this interface, then it might be wise to use a shared_ptr rather than a unique_ptr in serinfo, and pass a copy of that to every serpath you create. Copying shared_ptr is cheap (it’s actually destroying shared pointers that is fairly costly, but not really that much). That way, even if the original serinfo is destroyed, the serpaths won’t dangle.
Alternately, perhaps, you could make serpath non-copyable and non-movable, then create an array of them at construction time, and return references to them. Or, even simpler, just return a reference to the Dl_serpath struct directly. Because why not? All you’re doing with serpath is converting the char* to a string_view… and you don’t really gain all that much by that. (In fact, you lose info, because it’s very likely the user might want to use the returned path(s) with some system call or other, but you can’t use string_view because it isn’t NUL-terminated. You’d need to convert the string_view to a string to do pretty much anything useful with it anyway.)
Frankly, I’d just use a string (or possibly a std::filesystem::path!) in serpath, and all of this risk and hassle vanishes. Yeah, sure, you’re paying for the cost of a string allocation… but compared to all the costs involved in the allocation for serinfo, all those system calls, and so on… meh. The safety and ease of use is so much more worth it compared to avoiding a single, likely small allocation on what is certainly never going to be a hot path.
class serpath {

    // ... [snip] ...

    std::string_view name_;
    unsigned int flags_;

    // ... [snip] ...

public:
    std::string_view name() const { return name_; }
    unsigned int flags() const { return flags_; }

Are getters really necessary? Why couldn’t you just do:
class serpath
{
    friend class serinfo;

    explicit serpath(::Dl_serpath path)
        : name(path.dls_name)
        , flags(path.dls_flags)
    {}

public:
    std::string name; // perhaps better: std::filesystem::path path;
    unsigned int flags;
};

There doesn’t seem to be any gain from using getter functions for this simple return-value struct.
class serinfo

The thing that strikes me most about this class is that it’s actually a container of serpaths. All of the gymnastics you do in the constructor is really just to get that array of Dl_serpath, then dole it out safely. In theory, the class could be this:
class serinfo
{
    std::vector<serpath> _paths; // assuming serpath uses string, not string_view

public:

    explicit serinfo(void* dlhandle)
    {
        // everything the same as your current constructor, except `bytes` is
        // a local variable

        // then:

        _paths.reserve(bytes->dls_cnt);
        for (auto i = 0u; i < bytes->dls_cnt; ++i)
            _paths.push_back(serpath{bytes->dls_serpath[i]});
    }

    auto size() const noexcept { return _paths.size(); }

    // same for all the other useful const vector functions:
    // begin, end, operator[], etc.
};

I’m not suggesting it should be like this; there’s no point in allocating the Dl_serinfo structure, then just duplicating that allocation in the vector to copy all the data over. I’m just illustrating that it could be like this to make the point that serinfo is basically an immutable container of serpaths… so its interface should reflect that.
If you did implement a container interface, you could do stuff like:
// printing all the search paths
for (auto const& [path, _] : dl::get_search_paths_for(dlhandle))
    std::cout << path << '\n';

Note that you don’t need the entire container interface. All you need are const versions of begin(), end(), size(), and maybe empty(). If you wanted, you could add operator[] for convenience, but that’s pretty much it.
bytes = std::make_unique<std::byte[]>(size.dls_size);

This might work… if alignof(::Dl_serinfo) >= __STDCPP_DEFAULT_NEW_ALIGNMENT__.
See, std::malloc() always allocates with alignment alignof(std::max_align_t). Presumably ::Dl_serinfo doesn’t have any special alignment requirements (you don’t need to use std::aligned_alloc() to allocate it), so std::malloc() will never allocate under-aligned memory for ::Dl_serinfo, though it may allocate over-aligned memory, but that’s not a problem (other than a tiny amount of wasted memory, I suppose).
But new uses __STDCPP_DEFAULT_NEW_ALIGNMENT__… which may be more or less than alignof(std::max_align_t). If you did new ::Dl_serinfo directly, there would be no problem; if alignof(::Dl_serinfo) > __STDCPP_DEFAULT_NEW_ALIGNMENT__, then new would automatically fix the alignment for you.
But you’re doing new std::byte[sizeof(::Dl_serinfo)] (basically). You’re not allocating a ::Dl_serinfo, you’re allocating bytes (which have an alignment of 1, though new will naturally use __STDCPP_DEFAULT_NEW_ALIGNMENT__). So you need to be sure that the memory is properly aligned.
You could just use a static assert like:
static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= alignof(::Dl_serinfo));

That will just kill the compilation if there is a danger of alignment problems.
A better solution is to make sure the alignment is correct:
// in constructor:
constexpr auto alignment = std::align_val_t{alignof(::Dl_serinfo)};

bytes.reset(new (alignment) std::byte[size.dls_size]);
// note: can't use make_unique(), but doesn’t really matter

All the calls to ::dlinfo() look like this:
if (::dlinfo(/*...*/)) {
    throw dl_error(::dlerror());
}

For starters, I think you should make the error class more ergonomic:
if (::dlinfo(/*...*/)) {
    throw error{};
}

But also, simply converting the result of the ::dlinfo() call to bool doesn’t really give the best sense of what is actually going on here. I have no way of knowing whether maybe ::dlinfo() actually returns bool or something, or who knows what else, or what it means. You should generally not add redundant != or == to conditions… but the guideline is actually more subtle; it says adding redundant tests is unwise when there are only two meaningful values (like a valid pointer value and nullptr). With integers, there are often several meaningful values. It would be more clear to explain that you’re explicitly testing for “not zero” like this:
if (::dlinfo(/*...*/) != 0) {
    throw error{};
}

When I read that, even without looking up the ::dlinfo() API, I immediately understand, okay, this function returns zero on success.
One final thing: I’m not a fan of constructors that do more than just construct objects. serinfo’s constructor is more than just a constructor of serinfo objects. It’s actually doing a series of queries first, and then using the results to construct the object. That’s a violation of the single-responsibility principle.
Besides, you’re “doing a thing”… not “making a thing”. You’re “getting the search paths for a shared object”… you’re not “making a serinfo object”. The latter only happens as a side-effect of the former, but it’s not really the serinfo object you want… it’s the paths.
“Doing a thing” usually means you want a function, not a constructor. I think that’s especially important when you’re doing a thing that might fail, because then you can give the user the option of avoiding exceptions in cases where they can’t afford them. In other words, you could do:
class serinfo
{
private:
    explicit serinfo(std::unique_ptr<std::byte[]> data) :
        _data{std::move(data)}
    {}

    std::unique_ptr<std::byte[]> _data;

    friend auto get_search_paths_for(handle h) -> expected<serinfo>;

    // ... [snip] ...
};

// using something similar to the proposed std::expected:
auto get_search_paths_for(handle h) -> expected<serinfo>
{
    auto size = ::Dl_serinfo{};

    if (::dlinfo(h, RTLD_DI_SERINFOSIZE, &size) != 0)
        return make_unexpected(dl_error{});

    auto buffer = std::unique_ptr<std::byte[]>{};
    try
    {
        auto p = new (std::align_val_t{alignof(::Dl_serinfo)}) std::byte[size.dls_size];

        buffer.reset(p);
    }
    catch (...)
    {
        return make_unexpected(std::current_exception());
    }

    if (::dlinfo(h, RTLD_DI_SERINFOSIZE, buffer.get()) != 0)
        return make_unexpected(dl_error{});

    if (::dlinfo(h, RTLD_DI_SERINFO, buffer.get()) != 0)
        return make_unexpected(dl_error{});

    return serinfo{std::move(buffer)};
}

With usage like:
// I absolutely cannot tolerate an exception here:
if (auto const search_paths = dl::get_search_paths_for(dlhandle); search_paths)
{
    // do something with *search_paths
}
else
{
    // report error somehow
}

// Exceptions are okay here:
auto const search_paths = *dl::get_search_paths_for(dlhandle);
// if there was an error, it will be automatically rethrown

Once you do this, you open the door to allowing no-fail default-constructed serinfo objects, which internally just have a null pointer, but externally present as an empty list of search paths. And once you have that, you can trivially implement no-fail moving and swapping, which are very handy in general. Plus, being able to default construct things is handy in a lot of ways (like reading stuff from streams, for example).
Even if you don’t use something like expected<T> and just throw exceptions from the function, I think it’s still a better interface than doing all the work in the constructor on top of actually constructing the object.


Execute external file from inside C++ I am trying to create an application that on the front end presents the user with a text editor to input code into and then I will run that code and return the result. I thought it would be a fun project to try and build my own version of leetcode as a learning project.
Right now this is what I am doing to run the provided code. Let's say we are running python code, because that's all I have implemented right now.
First I take in the code that the user submits and create an a file that contains the given code:
std::string python(std::string code){
    std::string langCommand = "python3 ";
    std::string outFile;

    //I am hoping to parallelize this operation so I add threadID to output
    outFile = createOutFileName("PythonRunner.py");

    std::ofstream output;
    output.open(outFile);
    output << code;
    output.close();
    return langCommand + outFile;
}

The next thing I do is create an output file and run the previously created file but I send my stdout/stderr to another outputfile:
std::string Program::run(){
    std::string command = createFile(this->lang, this->code);

    this->outputFile = createOutFileName("output.txt");

    std::stringstream newCommand;
    newCommand << command;
    newCommand << ">> ";
    newCommand << outputFile;
    newCommand << " 2>&1";

    system(newCommand.str().c_str());
    std::string output = getOutputFileData(this->outputFile);
    cleanupFiles(command);
    return output;
}

Finally I return whatever I got from my output file and that is how I am executing my code.
I gotta think there is an easier way to do this. Especially since I am doing so much writing to a file and then reading from it is there anyway to get rid of that?
I also want to include more than one language in the future so I don't want to use any libraries that are specific to a certain language.
Lastly, this is my first C++ project so I would love any C++ tips!
Edit: I do want to eventually parallelize this code and find some way to encapsulate the program so it can't damage the system it is running on. If there is maybe some external program that would be good for that let me know and also gives me its stderr/stdout let me know.
Edit: As someone asked, here is the entire repo
https://github.com/lkelly93/coderunner

Rather than system() you should popen().
The difference is that system runs the command in a sub process with no access to this processes, while popen runs the command in a sub processes but provides accesses to the input and output streams of the sub processes.
This will allow you to run the sub-processes and stream input to the processes directly (from the input field you provided for standard input) and then read output from the processes and write it to the output field in your user interface.
FILE*  proc = popen(command);
std::string inputFromUser = getUserInputFromUI();
// Using fwrite() correctly left to user.
// You need to check for errors and continue etc.
fwrite(inputFromUser.c_str(), 1, inputFromUser.size(), proc);

char  buffer[100];
std::size_t size;
while((size = fread(buffer, 1, 100, proc)) != 0) {
    // Check for read errors here.
    sendToUserInterface(std::string(bufffer, buffer + size));
}

pclose(proc);

Sorted of related you don't need to save your pythong script as a file. The python command accept the - as a name which means read the script from the standard input rather than from the named file.
So you can run the python command (with popen()) then write the script you want to execute to the input stream of the file produced.
This will remove the need for any intermediate files.


FFmpeg command line for showing two videos side by side Here is command line for playing two videos side by side in sync in FFmpeg (ffplay). It can be useful for comaring videos, for example.

ffplay -v warning -f rawvideo -s 800x400 -i /dev/zero -vf 'movie=video1.mkv,scale=400x400 [mv2] ; movie=video2.mkv,scale=400x400 [mv1]; [in][mv1] overlay=0:0 [tmp]; [tmp][mv2] overlay=400:0'

Is there some better way, in particular:

Without using fake video input;
More easily extendible, without inventing names for [tmp];
Maybe with automatic geometry calculation, without manual coordinates;
Input video filenames should be more easily visible and together?

Partial result

ffmpeg -v warning -i video1.mkv -i video2.mkv -filter_complex '[0:v]scale=400:400,pad=800:400 [0:q]; [1:v]scale=400:400[1:q]; [0:q][1:q]overlay=400:0' -f nut -c:v rawvideo -c:a copy - | mplayer -noconsolecontrols -cache-min 1 -cache 1024000 -

Dummy video input removed
Filenames now separate command line aguments, not parts of a longer string
Still manual geometry
Still not automatically extendible to more videos
Preparing and playing video decoupled, now can use other player instead of mplayer (unfortunately mpv or vlc fails to provide seeking in this scenario)
Audio is preserved


Create a new unique file For debugging purposes I am storing requests into files. This code is supposed to create a new file for each request.

Somehow it doesn't look very nice, can it be improved? Additionally it would be nice if it could be made atomic.

filenamePrefix := exeDir + "/debug/" + time.Now().Format("20060102150405")
var filename string

for i := 0 ; ; i++ {
    if (i > 100) {
        log.Print("Could not find free filename")
        return
    }

    filename = filenamePrefix + "-" + strconv.Itoa(i)
    _, err := os.Stat(filename);
        if os.IsNotExist(err) {
        break
    }
}

fd, err := os.Create(filename)
if err != nil {
    log.Print(err.Error())
    return
}
defer fd.Close()

io.Copy(fd, request.Body)

Your code has a few items in it which concern me in a few small ways, and, in general, while I think your code is OK functionally, it could be done better (I am biased, I have done similar things, and ended up doing it differently).

First up, my concerns...

Timestamp function

while it is a 1-liner, I would still have a method to encapsulate the time formatting:

func buildFileName() string {
    return time.Now().Format("20060102150405")
}

filepath

You should use path/filepath to manage your path names:

import (
  "path/filepath"
)

filenamePrefix := filepath.Join(exeDir, "debug", buildFileName())

Functions

You need more functions... still. I would create a function to control the file creation process (note the use of %d instead of string.Itoa...):

func pickFileName(prefix string) (string, error) {
    for i := 0; i < 100; i++ {
        fname := fmt.Sprintf("%s_%d", prefix, i)
        if _, err := os.Stat(fname); os.IsNotExist(err) {
            return fname, nil
        }
    }
    return "", fmt.Errorf("Unable to create a unique file with the prefix %v in 100 tries", prefix)
}

Result

OK, now your code looks like:

filenamePrefix := filepath.Join(exeDir, "debug", buildFileName())
filename, err := pickFileName(filenamePrefix)
if err != nil {
    log.Print(err)
    return
}

fd, err := os.Create(filename)
if err != nil {
    log.Print(err)
    return
}
defer fd.Close()

io.Copy(fd, request.Body)

Error handling

You have log.Print(err.Error()) but log.Print(err) is fine. Still, you should probably be returning an error, instead of just logging it....

Alternative

If it were me, I would do two things.....

First up, create a directory for the actual process you are running as.

myid := fmt.SPrintf("Process_%d", os.Getpid())

then, ensure you use that in your path:

filenamePrefix := filepath.Join(exeDir, "debug", myid, buildFileName())

Now, you have a folder dedicated to the current process (and collisions with other running instances of your process are impossible - at the same time - though there may be an old instance of that same folder for an older process - that's OK).

Then, create a counter (a "global") too:

var debugCounter uint64

And a matching function:

func nextDebugId() string {
    return fmt.Sprintf("%d", atomic.AddUint64(&debugCounter, 1))
}

Now, you can simplify the whole thing (by modifying buildFileName) to:

func buildFileName() string {
    return time.Now().Format("20060102150405") + "_" + nextDebugId()
}

and your whole code then becomes:

fullpath := filepath.Join(exeDir, "debug", myid, buildFileName())

fd, err := os.Create(fullpath)
.....


How can I make this XML gallery snippet prettier? Everything works, i just want to know if i can change the way the xml tags are retreived. This way seems a bit bulky - but i could be wrong. Thoughts?

    var d = 0

    $(".timeline").children().eq(d).addClass("active")

    // default traverse function to buttons, inject node
    prev.data("node", array.length-1)
    next.data("node", 1)

    // use default node to get default properties
    src = xml.getElementsByTagName("src")[d].childNodes[0].nodeValue
    name = xml.getElementsByTagName("name")[d].childNodes[0].nodeValue
    date = xml.getElementsByTagName("date")[d].childNodes[0].nodeValue
    desc = xml.getElementsByTagName("desc")[d].childNodes[0].nodeValue
    img = '<img alt="'+name+'" src="'+src+'">'
        // inject default image properties
        $(".stage").html(img)
        $(".window").width($(".stage").width())
        $(".name").html(name), $(".date").html(date), $(".desc").html(desc)

There's a lot of duplication to get src, name, and so on. Simply use $.map:

var properties = $.map(["src", "name", "date", "desc"], function(id) {
  return xml.getElementsByTagName(id)[d].childNodes[0].nodeValue;
}

You can then use properties[0]. Not sure how I could return an object with the correct keys as simply as this. Maybe something like

var properties = {}
$.each(["src", "name", "date", "desc"], function(i, name) {
  properties[name] = xml.getElementsByTagName(name)[d].childNodes[0].nodeValue;
}

Don't forget any var
Create img using this less error-prone technique:

var img = $('<img>').attr({'alt': properties["name"], 'src': properties["src"]});


Running a simulation for all unique combinations of a range of parameters I have written a script which is supposed to call the function at the bottom for each unique combination of values in the parameter vectors. That function is a simulation which I intend to run over all the unique combinations of those parameters. The simulation is run on a cluster, and the sim_ind for each of these jobs is pulled from the environment.

Currently, my solution is to build a matrix with each row corresponding to a unique set of parameters. Then the sim_ind value (passed from the environment) is used to determine which unique set of parameters will be used in that iteration by indexing the matrix.

The following is the code that I am using, which works:

% getting sim_ind from env
AI = getenv('PBS_ARRAYID');
sim_ind = str2num(AI);

time_slots = 40000;

% parameter vectors
up_down_ratio_arr = [1];
traffic_load_arr = [ 0.01, 0.05, 0.1, 0.5, 1, 2, 5, 10, 100, Inf ];
blocked_modes_ind_arr = [1 2 3];
traffic_model_arr = [1 2];

% total number of unique combinations
uniq = length(up_down_ratio_arr) * length(traffic_load_arr) * length(blocked_modes_ind_arr) * length(traffic_model_arr);
parameters = zeros(uniq, 4);

% propagating the parameter list matrix
counter = 0;
for i = up_down_ratio_arr
    for j = traffic_load_arr
        for k = blocked_modes_ind_arr
            for l = traffic_model_arr
                counter = counter + 1;

                parameters(counter, 1) = i;
                parameters(counter, 2) = j;
                parameters(counter, 3) = k;
                parameters(counter, 4) = l;
            end
        end
    end
end

% wrapping the sim_ind
ind = mod(sim_ind, uniq);

% mod maps all multiples of the largest index to zero
if ind == 0
    ind = uniq;
end

% retrieving a parameter set
up_down_ratio = parameters(ind,1);
traffic_load = parameters(ind,2);
blocked_modes_ind = parameters(ind,3);
traffic_model = parameters(ind,4);

% call the routine
MainCode_Single_Backahul_Cell_Sch(time_slots, up_down_ratio, traffic_load, blocked_modes_ind, traffic_model, ceil(sim_ind/uniq) );

Is there a better way?

I have a version in which I use mod to map each value of the sim_ind parameter to a value that exists in the range of each vector. The problem with this solution was that not all the combination were unique. For example, in the following code traffic_load_arr is of size 10, and traffic_model_arr is of size 2. Therefore, whenever the 8th element of traffic_load_arr is selected; i.e. mod(sim_ind, 10) == 8, then the 2nd element of traffic_model_arr must also be selected. As a results, the parameter combination of traffic_load == 10 and traffic_model == 2 will never be formed.

I would appreciate for all code to be cross compatible with MATLAB and Octave.

For a full factorial experiment, you may want to avoid constructing the whole matrix just to get the factor levels for a single experiment, especially if you have a large number of factors and levels and you are using a language where loops are not very efficient. Here is how you can do it using mod.

In a single factor experiment, you run the experiment over the range of values of the first factor. This is easy to do using mod: use mod(sim_ind, nlevels_factor1), or

mod(sim_ind-1, nlevels_factor1) + 1

if you want to start at one (since MATLAB indexes arrays from 1).

To extend the experiment to a second factor, you want this new factor to have its first level value for the first nlevels_factor1 runs, then its second level value for the second nlevels_factor1 runs, etc... all the way up till the nlevels_factor1*nlevels_factor2th run.

You'll use mod again, but now you want the first argument to mod to be 1 for the first nlevels_factor1 runs, 2 for the next nlevels_factor1 runs, etc. So instead of passing the sim_index (or sim_index-1) to mod, you'll pass ceil(sim_index/nlevels_factor1), which has value 1 for the first nlevels_factor1 runs, 2 for the next nlevels_factor1 runs, etc. Your factor level for the second factor will be

mod(ceil(sim_index/nlevels_factor1) - 1, nlevels_factor2) + 1

if you want to start from 1.

You can continue this procedure to extend your experiment to any number of factors. Just make the divisor inside the ceil equal to the product of the number of levels of all of the previous factors.

Here's what it might look like for your example:

% getting sim_ind from env
AI = getenv('PBS_ARRAYID');
sim_ind = str2num(AI);

time_slots = 40000;

% parameter vectors
up_down_ratio_arr = [1];
traffic_load_arr = [ 0.01, 0.05, 0.1, 0.5, 1, 2, 5, 10, 100, Inf ];
blocked_modes_ind_arr = [1 2 3];
traffic_model_arr = [1 2];

size_up_down_ratio = length(up_down_ratio_arr);
size_traffic_load  = length(traffic_load_arr);
size_blocked_modes = length(blocked_modes_ind_arr);
size_traffic_model = length(traffic_model_arr);

% total number of unique combinations
uniq = size_up_down_ratio*size_traffic_load*size_blocked_modes*size_traffic_model;

% repeat each value once: 1,2,3,4,5,6,7,8,9,10,1,2,3,4...
i_up_down_ratio = mod(sim_ind - 1,size_up_down_ratio) + 1;
% repeat each value size_so_far times:
% 1,1,1,1,1,1,1,1,1,1,2,2,2,2,2,2,2,2,2,2,3,3,3,3,3...
size_so_far = size_up_down_ratio;
i_traffic_load  = mod(ceil(sim_ind/size_so_far) - 1, size_traffic_load) + 1;
size_so_far = size_so_far*size_traffic_load;
i_blocked_modes  = mod(ceil(sim_ind/size_so_far) - 1, size_blocked_modes) + 1;
size_so_far = size_so_far*size_blocked_modes;
i_traffic_model = mod(ceil(sim_ind/size_so_far) - 1, size_traffic_model) + 1;

up_down_ratio = up_down_ratio_arr(i_up_down_ratio);
traffic_load  = traffic_load_arr(i_traffic_load);
blocked_modes_ind = blocked_modes_ind_arr(i_blocked_modes);
traffic_model = traffic_model_arr(i_traffic_model);

% call the routine
MainCode_Single_Backahul_Cell_Sch(time_slots, up_down_ratio, traffic_load, blocked_modes_ind, traffic_model, ceil(sim_ind/uniq) );


Find the thousandth integer without the digits 3, 4, and 7 I have to find the thousandth number (excluding numbers 3, 4, 7) but it's taking a long time, about 9 seconds. I'd like to find out how to improve performance.

import java.sql.Date;
import java.text.DecimalFormat;
import java.text.SimpleDateFormat;

public class FirstChallange {

    private static int removedNumbers;
    private static int numberQuantity;
    private static int lastNumberFound;
    private static int cont;
    private static int pos3;
    private static int pos4;
    private static int pos7;

    public static void main(String[] args)
    {

        long inicio = System.currentTimeMillis();

        removedNumbers = 0;
        numberQuantity = 10000000;

        for (cont = 1; removedNumbers <= numberQuantity; cont++) {
            String str = new String();
            str = String.valueOf(cont);

            pos3 = str.indexOf("3");
            pos4 = str.indexOf("4");
            pos7 = str.indexOf("7");

            if((pos3 == -1) && (pos4 == -1) && (pos7 == -1)) {
                removedNumbers++;
                if(removedNumbers == numberQuantity){ // can not find numbers (3, 4, 7)
                    lastNumberFound = cont;
                }
            }
        }

        DecimalFormat dfmt = new DecimalFormat("0");

        System.out.println(dfmt.format(lastNumberFound));

        long fim  = System.currentTimeMillis();
        System.out.println(new SimpleDateFormat("ss.SSS").format(new Date(fim - inicio)));

    }
}

Is converting numbers into string and removing them with indexOf the best mode? or is there anything better than indexOf like RabinKarp?

Expected result: 180999565 in 5/4 seconds

import java.sql.Date;

You shouldn't need anything from java.sql unless you're using a relational database. java.util.Date would be more appropriate here (or nothing at all).

public class FirstChallange {

Challenge has one a and two es.

The name is not going to be very helpful in identifying the purpose of the class in six months' time, and there's no comment to say what the source of the challenge was either.

    private static int removedNumbers;
    private static int numberQuantity;
    private static int lastNumberFound;
    private static int cont;
    private static int pos3;
    private static int pos4;
    private static int pos7;

Variables should usually be in the narrowest scope possible. This means that static fields should be extremely rare.

        long inicio = System.currentTimeMillis();

It's best to be consistent in the use of language: either name all the variables in Portuguese or name them all in English.

This line has trailing whitespace. If you can configure your IDE to automatically remove this, you will reduce the number of spurious changes which muddle revision control diffs.

        DecimalFormat dfmt = new DecimalFormat("0");

        System.out.println(dfmt.format(lastNumberFound));

That is heavily overkill for System.out.println(lastNumberFound);

        removedNumbers = 0;
        numberQuantity = 10000000;

        for (cont = 1; removedNumbers <= numberQuantity; cont++) {
            String str = new String();
            str = String.valueOf(cont);

            pos3 = str.indexOf("3");
            pos4 = str.indexOf("4");
            pos7 = str.indexOf("7");

Actually iterating over objects is rarely the best way to count them. Iteration with filtering even less so. If you spend some time thinking through the mathematics before you start writing code, there's an easy way to tackle this problem by base conversion which takes about 20 milliseconds.


Nested Lists from HackerRank I am a beginner in python. This is the Nested Lists problem from HaackerRank.

  Given the names and grades for each student in a Physics class of N
  students, store them in a nested list and print the name(s) of any
  student(s) having the second lowest grade.

  Note: If there are multiple students with the same grade, order their
  names alphabetically and print each name on a new line.

I want to improve my code with better functions available in python and I want to decrease lines of code.

if __name__ == '__main__':
    scorecard = []
    for _ in range(int(input())):
        name = input()
        score = float(input())
        scorecard.append([name, score])
    scorecard.sort(key = lambda x:x[1])
    names = []
    lowest_score = scorecard[0][1]
    second_lowest = 0
    for i in range(len(scorecard)):
        if scorecard[i][1] > lowest_score and second_lowest == 0:
            second_lowest = scorecard[i][1]

        if second_lowest != 0 and scorecard[i][1] == second_lowest:
            names.append(scorecard[i][0])

    names.sort()

    for name in names:
    print(name)

I'd recommend separating the logic of getting the user input from the logic of building and sorting the nested list.
Your code doesn't build a nested list in a way that's useful for solving the problem; I think what you want is a list of lists of names, where each list of names corresponds to a grade.
Any time you want to group things into buckets (e.g. all the names that go with a grade), it's a good time to use a dictionary!
Using list comprehensions is frequently a good way to express a loop in a shorter way.
Type annotations make it easier to keep track of what your code is doing (and they let you use mypy to catch bugs).

Here's my solution that uses a separate function to build the nested list, using a dictionary as an intermediate step to collect all the names for a given grade.  Note that the type annotations tell you exactly where the names (strings) and grades (floats) are going at each step!

from collections import defaultdict
from typing import Dict, List, Tuple

def build_grade_lists(grades: List[Tuple[str, float]]) -> List[List[str]]:
    """
    Given a list of (name, grade) tuples, return a list of names for each grade.
    Each list is sorted, i.e.:
        the top level list is sorted from lowest to highest grade
        each list of names is sorted alphabetically
    """
    grade_lists: Dict[float, List[str]] = defaultdict(list)
    for name, grade in grades:
        grade_lists[grade].append(name)
    return [sorted(grade_lists[grade]) for grade in sorted(list(grade_lists.keys()))]

if __name__ == '__main__':
    scorecard = [(input(), float(input())) for _ in range(int(input()))]
    for name in build_grade_lists(scorecard)[1]:
        print(name)