Finding Memoryleaks In Data Structures Looking for constructive criticism on my code below for detecting a memory leak in a data structure.

    MemoryLeak = {

  uniq_id: (new Date()).getTime(),
  checked: 1,
  is_seen: [],

  checkLeaks: function(obj) {
    var self = MemoryLeak

    if(!obj || (typeof obj == 'function') || self.checked > 20000)
      return;

    if ((self._isArray(obj) || self._isObject(obj))) {
      if (self._isArray(obj)) {
        self._logTooBig(obj, obj.length)
        for (var i = 0; i < obj.length; i++) {
          self._checkIfNeeded(obj[i])
        }
      }
      else if (self._isObject(obj)) {
        self._logTooBig(obj, self._keys(obj).length)

        for (var key in obj) {
          self._checkIfNeeded(obj[key])
        }
      }
    }
  };

  _checkIfNeeded: function(obj) {
    if (!obj)
      return;

    var self = MemoryLeak;
    self.checked++

    if ((self._isArray(obj) || self._isObject(obj))) {
      if(obj.__leaks_checked == self.uniq_id)
        return;
      obj.__leaks_checked = self.uniq_id

      setTimeout(self._partial(self.checkLeaks, obj), 5);
    }
  };

  _logTooBig: function(obj, limit) {
    if (limit > 200) {
      console.log('Object too big, memory leak? [size: ' + limit + ']')
      console.log(obj)
      console.log('-------')
    }
  };

  _keys: function(obj) {
    var rval = [], prop
    for (prop in obj)
      rval.push(prop)
    return rval;
  };

  _isArray: function(obj) {
    try {
      return obj instanceof Array
    }
    catch(e) {
      return false;
    }
  };

  _isObject: function(obj) {
    return (typeof obj == 'object')
  };

  _partial: function(fn) {
    var args = Array.prototype.slice.call(arguments)
    args.shift()
    return function() {
      var new_args = Array.prototype.slice.call(arguments)
      args = args.concat(new_args)
      return fn.apply(window, args)
    }
  }
}

_logTooBig: function(obj, limit) {
  if (limit > 200) {
    console.log('Object too big, memory leak? [size: ' + limit + ']')
    console.log(obj)

Naming: Surely the parameter you've called limit ought to be named size! The word "limit" implies an upper bound, as in "speed limit". In this case, the "limit" on object size is 200. The parameter you've currently called limit is the thing you're supposed to be comparing to the size limit. What it actually represents is just a size.

Separately, are you at all worried that repeatedly console.logging (only) huge objects will be a bad debugging experience? Or do good browsers already handle huge objects just fine?

var self = MemoryLeak;
self.checked++

Syntax nit: missing semicolon after ++. This is legal of course, but I think many linters will flag it as an inconsistency.

Why do you do this thing with self = MemoryLeak instead of just saying this.checked++? I might be exposing my own lack of knowledge here, but I thought this was supposed to do the right thing in "member" functions called with the MemoryLeak.foo() syntax. Is your use of self actually working around a problem with this, or is it just an idiosyncrasy that could be cleaned up?

try {
  return obj instanceof Array
}
catch(e) {
  return false;
}

Again I may be exposing my own lack of knowledge, but I'm about 75% sure that instanceof cannot possibly throw an exception. What case are you worried about here?

  _keys: function(obj) {
    var rval = [], prop
    for (prop in obj)
      rval.push(prop)
    return rval;
  };

This should at least be rewritten

  _keys: function(obj) {
    var rval = [];
    for (var prop in obj)
      rval.push(prop);
    return rval;
  };

and (unless I'm missing something) it should simply be

  _keys: function(obj) { return Object.keys(obj); }

if(!obj || (typeof obj == 'function') || self.checked > 20000)
  return;

I see where you initialize checked = 1 and where you checked++, but I don't see anywhere that checked's value decreases or is reset. So do you only ever check 20000 entities and then stop checking forever? That doesn't seem like super useful behavior, since memory leaks typically manifest only after a fairly long time. What's the point of the checked counter? (If you keep it, then it would be a good idea to add a //code comment explaining why it's desirable. Code comments are great for explaining the "why" of code.)

Speaking of "explain the why": it would have been nice to see an example of how you expect the user to use this leak checker. I gather that he'd just insert a call to MemoryLeak.checkLeaks(mySuspiciousObject) after initializing the object, and that would trigger the checking of mySuspiciousObject every 5 seconds for 20000 iterations?

Certainly exposing my ignorance: I wasn't aware that all your };s were legal. I thought you'd have to make them },.


Safely Initialising a static variable to a file This might appear to be a trivial task, but I think it's not the case. My code originally looked like this:

class DataSetToPdf
{
  static byte [] F1 = Util.GetFile( "C:\PdfFiles\FreeSans.ttf" );
  public static void Go( Data.DataSet ds, int ix, System.IO.Stream output )
  {
    // Code which uses F1
  }
}

Util.GetFile simply reads the file and returns an array of bytes. For completeness, here it is:

class Util
{
  public static byte[] GetFile( String path )
  {
    IO.MemoryStream ms = new IO.MemoryStream();
    using( IO.FileStream f = IO.File.OpenRead( path) )
    {
      f.CopyTo( ms );
    }
    return ms.ToArray();
  }
}

However, when using DataSetToPdf in a multi-threaded environment (IIS), I was sometimes ( but not always ) getting mysterious null reference exceptions, which I could not easily debug. However, what I believe what was happening is that the static method could be called before the initialisation is complete. Instead what appears to be is necessary is this:

class DataSetToPdf
{

  static System.Object Locker = new System.Object();

  static byte [] F1;

  public static void Go( Data.DataSet ds, int ix, System.IO.Stream output )
  {
    lock( Locker )
    {
      if ( F1 == null )
      {
         F1 = Util.GetFile( "C:\PdfFiles\FreeSans.ttf" );
      }
    }
    // Code which uses F1
  }
}

Please review my updated code, was it wrong before, is it right now?

I would avoid doing this kind of thing in a static constructor or static initializer. Error handling in this situation can get tricky: errors thrown will result in a TypeInitializationException and the static constructor will not get executed again even if this is handled. You also don't have much control about where this will get thrown in your code: it would be very easy to refactor your code and end up with the first reference to the type appearing in a completely different place.

Loading from a file is also going to cause issues when you write tests: a reference to the type will trigger the static initialization and therefore depend on the presence of the file, which you will need to ensure is present in all the relevant test assemblies (and this kind of thing becomes a headache if you are using things like ReSharper's shadow copying behavior).

I would consider avoiding the use of static methods: define an interface and pass the relevant data in through the class constructor, e.g.

public interface IDataSetToPdf
{
    void Process();
}

public class DataSetToPdf : IDataSetToPdf
{
    private readonly byte[] fontData;

    public DataSetToPdf(byte[] fontData)
    {
        this.fontData = fontData ?? throw new ArgumentNullException(nameof(fontData));
    }

    public void Process()
    {
        // do stuff with fontData
    }
}

There are several advantages of doing this: firstly, if the data is in a static field, it it will last as long as the application does, whereas using an instance field allows the memory to be reclaimed by the garbage collector when no further references to it remains.

Another advantage is that it makes it easier to unit test: provided you write your code against the interface IDataSetToPdf rather than the concrete class DataSetToPdf, you would be able to provide a mock implementation of the interface for your tests, which would allow you to focus your tests on the calling code rather than the combination of the calling code with DataSetToPdf.


Computing function time in C++ I used the std::chrono library for computing the timing of a function and need some review on it.

Here is the code:

namespace gstd
{

#ifdef GTIME
    inline namespace gtime
    {
        template<typename type, typename period>
        using duration = std::chrono::duration<type, period>;
        std::function<std::chrono::time_point<std::chrono::steady_clock>( )>current_time = std::chrono::high_resolution_clock::now;

        template<typename RetType>
        std::optional<double> getFunctionTime(std::function<RetType(void)>function) {
            try {
                auto start_time = current_time( );
                function( );
                auto end_time = current_time( );
                duration<double, std::milli> dur = end_time - start_time;
                return dur.count( );
            }
            catch ( ... ) {
                return {};
            }
        }
        template <typename RetType, typename arg>
        std::optional<double> getFunctionTime1(std::function<RetType(arg)> function, arg value) {
            try {
                auto start_time = current_time( );
                function(value);
                auto end_time = current_time( );
                duration<double, std::milli> dur = end_time - start_time;
                return dur.count( );
            }
            catch ( ... ) {
                return {};
            }
        }
        template<typename RetType, typename arg1, typename arg2>
        std::optional<double> getFunction2(std::function<RetType(arg1, arg2)>function, arg1 firstparam, arg2 secondparam) {
            try {
                auto start_time = current_time( );
                function(firstparam, secondparam);
                auto end_time = current_time( );
                duration<double, std::milli> dur = end_time - start_time;
                return dur.count( );
            }
            catch ( ... ) {
                return {};
            }
        }
    }

#endif // GTIME
}

This is the code in the header file. Now how i am using it is here:

#define GTIME

#include "GStandard.h"
#include <iostream>

int number_crunching( ) {
    int sum { 0 };
    for ( int i = 0; i < 10000000; i++ )
        sum += 1;
    return sum;
}
int multiplier(int number) {
    for ( int i = 0; i < 10000000; i++ ) {
        number *= ( i + 1 );
    }
    return number;
}
int sum( int a, int b) {
    return a + b;
}
int main( ) {
    auto ftime = gstd::getFunctionTime<int>(number_crunching);
    auto f1time = gstd::getFunctionTime1<int, int>(multiplier, 5);
    auto f2time = gstd::getFunction2<int, int,  int>(sum, 4, 5);

    std::cout << ftime.value( ) << '\n';
    std::cout << f1time.value( ) << '\n';
    std::cout << f2time.value( ) << '\n';

    std::cin.get( );
    return 0;
}

Some suggestions were previously made on making it variadic but I am not doing it so that the students at my college can make improvement to this code. I am looking forward to removing the try catch block and if there are any more changes that you guys could suggest that would be very nice.

Be more generic

As I've suggested here, you should not force the use of an std::function. Instead, take any type of callable and either a fixed or an unlimited number of arguments, using std::invoke inside your implementation.

More importantly, you really don't need multiple functions for a different number of arguments; use variadic templates, e.g. have a signature such as:

template< class F, class... Args>
std::optional<double> getFunctionTime(F&& f, Args&&... args) noexcept;

(ignoring any other potential changes)

This will make it easier for you to invoke, too:

auto ftime = gstd::getFunctionTime(number_crunching);
auto f1time = gstd::getFunctionTime(multiplier, 5);
auto f2time = gstd::getFunctionTime(sum, 4, 5);

Alternatively, you could only accept 0-argument runnables, in which case you simply pass lambdas:

auto f1time = gstd::getFunctionTime([]() { return multiplier(5); });

Return a chrono::duration

Don't convert your duration to a plain numeric value - keep the more specific type.

Use integral values for durations

Why use a floating-point value in the duration? Stick with integral values unless you have a good reason to do otherwise.

Consider changing the name

I'd use time_an_invocation or time_invocation but I guess that's a matter of taste.

Consider leaving it to the caller to handle exceptions

You're forcing a certain behavior w.r.t. exceptions. I'd consider simply not handling them, so that timing the invocation of a function that throws an exception, throws an exception. That would simplify your code and avoid a  dependence on std::optional, which the caller might not be interested in. Remember the caller has to have additional code to handle the empty-optional case anyway.

Consider using a stopwatch class instead

In a related question, @Edward suggests keeping the invocation and the time measurement separate, using a "stop-watch" class. That might not be "better" but it's certainly worth considering.


Linked List implementation in Ruby I was working on Leet code design a Linked List problem. My solution with a @tail pointer, althought it added complexity it made adding to tail O(1), also having a variable size @size made it easier to counter edge cases. Here are some of the stats:

  Success Details Runtime: 100 ms, faster than 84.62% of Ruby online
  submissions for Design Linked List. Memory Usage: 9.6 MB, less than
  100.00% of Ruby online submissions for Design Linked List.

If there is any enhancements or feedback please share some. Could the Linked List be optimize? Are you able to understand how is design when reading the code?

class MyLinkedList
    attr_accessor :head, :size, :tail
=begin
    Initialize your data structure here.
=end
    def initialize()
      @head = nil
      @size = 0
      @tail = nil
    end

=begin
    Get the value of the index-th node in the linked list. If the index is invalid, return -1.
    :type index: Integer
    :rtype: Integer
=end
    def get(index)
      return -1 if @head == nil
      return -1 if index > @size - 1
      return @head.val if index == 0
      return @tail.val if index == @size - 1

      i = @head
      j = 0

      while i != nil
        if j == index
          return i.val
        end
        i = i.next
        j += 1
      end
      -1
    end

=begin
    Add a node of value val before the first element of the linked list. After the insertion, the new node will be the first node of the linked list.
    :type val: Integer
    :rtype: Void
=end
    def add_at_head(val)
      new_node = Node.new(val)
      new_node.next = @head
      @tail = new_node if @head == nil
      @head = new_node
      @size += 1
    end

=begin
    Append a node of value val to the last element of the linked list.
    :type val: Integer
    :rtype: Void
=end
    def add_at_tail(val)
      return add_at_head(val) if @head == nil
      node = Node.new(val)
      @tail.next = node
      @tail = node
      @size += 1
    end

=begin
    Add a node of value val before the index-th node in the linked list. If index equals to the length of linked list, the node will be appended to the end of linked list. If index is greater than the length, the node will not be inserted.
    :type index: Integer
    :type val: Integer
    :rtype: Void
=end
    def add_at_index(index, val)
      return add_at_head(val) if index == 0
      return add_at_tail(val) if @head == @tail
      return add_at_tail(val) if @size == index
      return if index > @size

      i = @head
      j = 0

      while i != nil
        if j == index - 1
          tmp = i.next
          node = Node.new(val)
          i.next = node
          node.next = tmp
          @size += 1
          return
        end
        i = i.next
        j += 1
      end

    end

=begin
    Delete the index-th node in the linked list, if the index is valid.
    :type index: Integer
    :rtype: Void
=end
    def delete_at_index(index)
      return if index < 0 || index > @size - 1
      return delete_at_head if index == 0
      return delete_at_tail if index == @size - 1

      i = @head
      j = 0

      while i != nil
        if j == index - 1
         i.next = i.next.next
         @size -= 1
         return
        end
        i = i.next
        j += 1
      end

    end

    def delete_at_head
      if @head == nil
        return nil
      elsif @head == @tail #one element in the linked list
        @head = nil
        @tail = nil
        @size -= 1
        return
      else
        @head = @head.next
        @size -= 1
      end
    end

    def delete_at_tail
      if @head == nil
        return nil
      elsif @head == @tail
        return delete_at_head
      end

      current = @head
      prev = nil

      while(current != tail)
        prev = current
        current = current.next
      end
      prev.next = nil
      @tail = prev
      @size -= 1
      return
    end

    private

    class Node
        attr_accessor :val, :next
        def initialize(val)
            @val = val
            @next = nil
        end
    end
end

My solution with a @tail pointer, althought it added complexity it made adding to tail O(1), also having a variable size @size made it easier to counter edge cases.

Having a tail pointer and a size variable makes totally sense.

  Could the Linked List be optimize?

I don't think you can improve performance much here, you already cover most / all of the corner cases. Well done to implement add_at_index with one iteration.

  Are you able to understand how is design when reading the code?

Here are some suggestions to make the code more readable

Extract helper methods

In your code you do several times if @head == nil to check if the list is empty. What about extracting an empty? method?

def empty?
  head.nil?
end

Same goes for instance with increasing the size.

  def increase_size
    @size += 1
  end

Use getter / setter methods

You already define getter and setter methods in your class but then fail to use them.

attr_accessor :head, :size, :tail

Instead of doing @head you should use head etc. Another problem is that the getter / setter are public, move them to the private section of the class. There is no need that consumer of this class know about these implementation details.

Implement the Enumerable interface

At several places in your code you need to iterate over your list with a while loop. If you implement the Enumerable interface of Ruby, you only need to do this once.

class MyLinkedList
  include Enumerable

  def initialize
    @head = nil
    @size = 0
    @tail = nil
  end

  def each
    current = head
    until current.nil?
      yield current
      current = current.next
    end
  end

  def [](index)
    each_with_index do |item, i|
      return item.val if i == index
    end
  end

https://ruby-doc.org/core-2.7.1/Enumerable.html

Use Ruby method names

You probably use the method stubs provided from LeetCode but I think it's still worth mentioning. Whenever possible you should use similar methods as already in the Ruby standard library. If we look at the Array class, we see for instance these methods.

def [](index) # get(index)
def unshift(value) # add_at_head(val)
def <<(value) # add_at_tail(val)
def append(value) # add_at_tail(val)
def [](index, value) # def add_at_index(index, val)
def shift # delete_at_head
def pop # delete_at_tail

Summary

Here are some of my suggestions applied

class MyLinkedList
  include Enumerable

  def initialize
    @head = nil
    @size = 0
    @tail = nil
  end

  def [](index)
    return if empty?
    return if index > size - 1
    return head.val if index.zero?
    return tail.val if index == size - 1

    each_with_index do |item, i|
      return item.val if i == index
    end
  end

  def unshift(value)
    node = Node.new(value)
    node.next = head

    @tail = node if empty?
    @head = node
    increase_size

    value
  end

  def <<(value)
    return unshift(value) if empty?

    node = Node.new(value)
    tail.next = node
    @tail = node
    increase_size

    value
  end

  def []=(index, value)
    return add_at_head(value) if index == 0
    return add_at_tail(value) if head == tail
    return add_at_tail(value) if size == index
    return if index > size

    each_with_index do |item, i|
      insert_node(item, value) if i == index - 1
    end
  end

  def each
    current = head
    until current.nil?
      yield current
      current = current.next
    end
  end

  private

  attr_accessor :head, :size, :tail

  def insert_node(item, value)
    tmp = item.next
    node = Node.new(value)
    item.next = node
    node.next = tmp
    increase_size
  end

  def empty?
    @head.nil?
  end

  def increase_size
    @size += 1
  end

  class Node
    attr_accessor :val, :next

    def initialize(val)
      @val = val
      @next = nil
    end
  end
end


What do you think about this uint64 random generator? I've looked for a good method to generate an integer inside a range of values.  I think using modulo is not a good method because the values are not perfectly mapped in the range.  So I mapped the pseudo random generated number in the range by using double variables types.

What do you think about this method?  I know that performance is not better when using a modulo. How would you improve the method or the performance by keeping a perfect match?

quint32 randquint32(quint32 low, quint32 high)
{
    const quint32 uint_max = 0xffffffff;

    if (low == 0 && high == 0)
    {
        high = uint_max;
        if (RAND_MAX > high)
        {
            return (qrand() & high);
        }
    }
    else if (low > high)
    {
        qSwap(low, high);
    }

    const quint8 numberOfBits = bitCount(RAND_MAX);
    quint32 myRand = 0;
    int i = qCeil(((double)32) / ((double)numberOfBits));
    while (i--)
    {
        myRand += qrand();
        if (i)
        {
            myRand <<= numberOfBits;
        }
    }

    if (low == 0 && high == uint_max)
    {
        return myRand;
    }
    if (myRand == uint_max)
    {
        return high;
    }

    const double uint_max_double = (double)uint_max;

    double range = high - low + 1;
    double tmpRand = ((double)myRand) / uint_max_double * range;
    return qFloor(tmpRand);
}

quint8 bitCount(quint32 n)
{
    quint8 count = 0;
    while (n)
    {
        if (n & 1)
        {
            count++;
        }
        n >>= 1;
    }
    return count;
}

NEW VERSION:

// count setted bits at the right of a mask
quint8 bitCountRightMask(quint64 mask)
{
    quint8 bc = 0;
    while (mask & 1)
    {
        mask >>= 1;
        bc++;
    }
    return bc;
}

const quint64 uint64_max = 0xffffffffffffffff;
const int bitCountRandMax = bitCountRightMask(RAND_MAX);

// randomise an uint64 variable
quint64 rand64()
{
    quint64 myRand = qrand();

    if (uint64_max > RAND_MAX)
    {
        int i = qCeil(64 / (double)bitCountRandMax) - 1;
        do
        {
            myRand <<= bitCountRandMax;
            myRand += rand();
        } while (i--);
    }

    return myRand;
}

// randomize an uint64 variable between low and high (fast method (modulo))
quint64 rand64(quint64 low, quint64 high)
{
    if (low == 0 && high == 0)
    {
        high = uint64_max - 1;
    }
    else if (low > high)
    {
        qSwap(low, high);
    }

    return low + (rand64() % (high - low + 1));
}

// randomize an uint64 variable between low and high (precise method (double))
quint64 prand64(quint64 low, quint64 high)
{
    if (low == 0 && high == 0)
    {
        return rand64();
    }
    else if (low > high)
    {
        qSwap(low, high);
    }

    quint64 myRand = rand64();

    if (myRand == uint64_max)
    {
        return high;
    }

    static double uint64_max_double = static_cast<double>(uint64_max);

    return low + (myRand / uint64_max_double * (high - low + 1));
}

DONT DO THIS

while (i--)
{
    myRand += rand();
    if (i)
    {
        myRand <<= numberOfBits;
    }
}

It seems like you are trying to increase the range of random numbers.
What you are doing is screwing up the distribution of the random numbers and thus making them less random. Have you tried plotting a histogram to show the distribution.

Luckily for you your system probably only has 32 bit rands and thus this is not changing anything; as the loop is only done once.

ASSUMPTION HERE

static const unsigned long uint_max = 0xffffffff;  // assuming that long is 32 bits

// long has at least 32 bits but can be larger.

Is this not the whole point of all the work to avoid this:

    if (high < RAND_MAX)
    {
        return (rand() & high);  // basically this is the same as using
    }                            // % high. You are folding at high
                                 // unless RAND_MAX is a multiple of high
                                 // then you have an uneven distribution.

There is plenty of documentation on the web of how to do this correctly please try and find rather than re-invent something like this. Random number generation is a non trivial task and exceedingly hard to get correct. Don't mess with it unless you have a PhD in probability.

Rand done properly: https://stackoverflow.com/a/10219422/14065


integer log base 10 function without using java.lang.Math? I am assigned to a task of creating a function int logBase10Estimate(int n) that returns the log of a number base 10. The answer is integer part only.(w/o using the Math class)

Is this okay?

static int logBase10Estimate(int n){
    int count = 0;
    for(int x = 10; ;x*=10){
        if(x < n){
            count+=1;
            System.out.println(x);
        }
        else if(x == n){
            count+=1;
            break;
        }
        else{
            break;
        }
    }
    return count;
}

You have the possibility of an infinite loop.

Consider: logBase10Estimate(2147483647). Should be about 9.  We need a value of \$10^i\$ which is greater than 1000000000.  So, what values does x take on?

         10
        100
       1000
      10000
     100000
    1000000
   10000000
  100000000
 1000000000
 1410065408
 1215752192
 -727379968
 1316134912
  276447232
-1530494976
 1874919424
 1569325056
-1486618624
  :  :  :

Any value of num greater than 1000000000 is going to give incorrect results.  And some values, like 2147483647 will never give an answer, because no int value is greater than it, and x will never be odd, so it will never be equal to it either.

Not only is slepic's solution cleaner, it is correct and it terminates for all values.


Greater on the right side in Java Given an array of integers, replace every element with the next
  greatest element (greatest element on the right side) in the array.
  Since there is no element next to the last element, replace it with
  -1.

  Input:

  The first line of input contains an integer T denoting the number of
  test cases. The first line of each test case is N,N is the size of
  array. The second line of each test case contains N input A[i].

  Output:

  Print the modified array.

  Constraints:

  1 ≤ T ≤ 50 1 ≤ N ≤ 100 1 ≤ A[i] ≤ 1000

  Example:

  Input:

2
6
16 17 4 3 5 2
4
2 3 1 9

  Output:

17 5 5 5 2 -1
9 9 9 -1

My approach:

/*package whatever //do not write package name here */

import java.io.InputStreamReader;
import java.io.BufferedReader;
import java.io.IOException;
import java.util.Collections;

class GFG {

    private static int [] calcNewArr(int [] arr)
        {
            int [] ans = new int[arr.length];
            ans[0] = getNextBiggest(arr,1);
            for (int i = 1; i < arr.length - 1; i++)
                {
                    ans[i] = getNextBiggest(arr,i+1);
                }
            ans[ans.length - 1] = -1;
            return ans;
        }

    private static int getNextBiggest (int [] arr, int start)
        {
            int max = Integer.MIN_VALUE;
            for (int i = start; i < arr.length; i++ )
                {
                    if ( max < arr[i])
                        {
                            max = arr[i];
                        }
                }
            return max;
        }

    public static void main (String[] args) throws IOException {
       BufferedReader br = new BufferedReader (new InputStreamReader(System.in));
       String line = br.readLine();

       int numTests = Integer.parseInt(line);
       String line2;
       String line3;
       int size;
       int [] arr;
       int [] result;

       for (int i = 0; i < numTests; i++)
        {
            line2 = br.readLine();
            size = Integer.parseInt(line2);
            line3 = br.readLine();
            String []inps = line3.split(" ");
            arr = new int[size];
            for (int j = 0; j < size; j++)
                {
                 arr[j] = Integer.parseInt(inps[j]);
                }
            result = calcNewArr(arr);
            for (int k = 0; k < size; k++)
                {
                    System.out.print(result[k] + " ");
                }
           System.out.println();
        }

    }
}

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

How can I further improve my approach?

2.Is there a better way to solve this question?
Are there any grave code violations that I have committed?
Can space and time complexity be further improved?

Reference

You're doing a lot of work to parse the input. It seems like it'd be easier if you just used the Scanner class:

final Scanner sc = new Scanner(System.in);
final int numTests = sc.nextInt();

for (int i = 0; i < numTests; i++) {
    final int size = sc.nextInt();
    final int[] arr = new int[size];
    final int[] result;

    for (int j = 0; j < size; j++) {
        arr[j] = sc.nextInt();
    }
    result = calcNewArr(arr);
}

Your implementation runs at \$O(n^2)\$, however, this should be possible in \$O(n)\$ time if you just looked through the array right to left. If the question was asking for left to right on a reverse of the array, the problem would be pretty obvious since it's basically just iterate through and apply some simple rules:

The first element is -1
Copy the highest number seen so far into the position
If the value replaced is greater, set it as the largest number seen so far

So, I think something like this fulfills your calcNewArray:

int largest = -1;

for (int i = ans.length - 1; i >= 0; i--) {
    final int value = ans[i];
    ans[i] = largest;
    largest = Math.max(largest, value);
}

Obviously, comments can improve the code and there's some stylistic things that'll make most Java devs raise their eyebrows, but it's not a big deal. One thing, in particular, however, is to consistently use the following format for array types:

int[] arr = new int[size];

As opposed to int [] arr or int []arr


Why is this divide and conquer algorithm inefficient? I was solving one of the problem in USACO 2007 Open Silver:

  Description

  Farmer John has taken his cows on a trip to the city! As the sun sets, the cows gaze at the city horizon and observe the beautiful silhouettes formed by the rectangular buildings.

  The entire horizon is represented by a number line with \$N (1 ≤ N ≤ 40,000\$) buildings. Building \$i\$'s silhouette has a base that spans locations \$Ai\$ through \$Bi\$ along the horizon \$1 ≤ Ai < Bi ≤ 1,000,000,000\$ and has height \$Hi\$ (\$1 ≤ Hi ≤ 1,000,000,000\$).

  Determine the area, in square units, of the aggregate silhouette formed by all \$N\$ buildings.

  Input

  Line 1: A single integer: N
  Lines 2...N+1: Input line i+1 describes building \$i\$ with three space-separated integers: \$Ai\$, \$Bi\$, and \$Hi\$.

  Output

  Line 1: The total area, in square units, of the silhouettes formed by all \$N\$ buildings

  Sample Input

4

2 5 1

9 10 4

6 8 2

4 6 3

  Sample Output

16

I used a divide-and-conquer algorithm to solve this problem, but failed. The feedback of the online judgement system is Time Limit Exceeded. Could you help me to make my code more efficient, readable, and clear?

#include <stdio.h>
#include <malloc.h>

#define INF 10000000000

enum {MAX = 2, DIM = 3};

typedef struct Outline Outline;
struct Outline {
    int pos;
    int height;
};

/* merge two outlines of buildings */
Outline *mergebuilding(Outline *a, Outline *b, int num)
{
    int a_move = 0;
    int b_move = 0;
    int r_count = 0;
    int r_move = 0;
    int pre = -1;
    int x = 0;
    int y = 0;
    Outline *r = (Outline *) malloc(MAX * num * (sizeof(Outline)) + sizeof(Outline));

    while (a[a_move].pos < INF || b[b_move].pos < INF) {
        r[r_move].pos = (a[a_move].pos <= b[b_move].pos) ? a[a_move].pos : b[b_move].pos;
        if (r[r_move].pos == a[a_move].pos) {
            x = a[a_move].height;
            a_move++;
        }

        if (r[r_move].pos == b[b_move].pos) {
              y = b[b_move].height;
             b_move++;
        }

        r[r_move].height = (x >= y) ? x : y;

        if (r[r_move].height != pre) {
            pre = r[r_move].height;
            r[r_move+1] = r[r_move];
             r_move++;
        }
    }
    r[r_move].pos = INF;

    return r;
}

/* divide-and-conquer */
Outline *recursive(Outline **p, int low, int high)
{
    /*
    Recursively divide the set of buildings into two parts
    until one block, then merge two blocks from the bottom to the top.
    */
    Outline *part1, *part2, *merge;
    int mid;

    if (low == high)
        return p[low];

    mid = (low + high) / 2;
    part1 = recursive(p, low, mid);
    part2 = recursive(p, mid+1, high);
    merge = mergebuilding(part1, part2, low+high+1);

    free(part1);
    free(part2);

     return merge;
}

int main()
{
    int **bul;
    int num;
    Outline **p, *cp;

    scanf("%d", &num);
    bul = (int **) malloc(num * sizeof(int));
    p = (Outline **) malloc(num * sizeof(Outline));

    for (int i = 0; i < num; i++) {
        bul[i] = (int *) malloc(DIM * sizeof(int));
        p[i] = (Outline *) malloc(DIM * sizeof(Outline));

        scanf("%d %d %d", &bul[i][0], &bul[i][1], &bul[i][2]);

        p[i][0].pos = bul[i][0];
        p[i][0].height = bul[i][2];
        p[i][1].pos = bul[i][1];
        p[i][1].height = 0;
        p[i][2].pos = INF;
        free(bul[i]);
    }
    free(bul);

    cp = recursive(p, 0, num-1);

    free(p);

    int temp, square;
    square = 0;

    for (int i = 0; cp[i].pos < INF; i++) {
        temp = (cp[i+1].pos - cp[i].pos) * cp[i].height;
        square += temp;
    }
    printf("%d\n", square);
    free(cp);

    return 0;
}

I don't know for sure why your program is running out of time (since I know nothing about the target platform), but my guess is that it's either item (1) or (2) in the list below. These errors lead to undefined behaviour, and then, well, anything could happen (that's what undefined means), including an infinite loop.

This line computes the wrong size for the allocation:

bul = (int **) malloc(num * sizeof(int));

You want an array of num pointers to integers but you are allocating space for num integers. So this will only work if an integer is the same size as a pointer. On my platform/compiler (x86-64/Clang) an integer is 4 bytes long but a pointer is 8 bytes long so this computes a size that is too small, and then the loop writes off the end of the array, which results in undefined behaviour. (On my platform this causes the program to crash with a segmentation fault.)

You need to write:

bul = malloc(num * sizeof(int *));

or:

bul = malloc(num * sizeof *bul);

instead. Note that there's no need to cast the result of malloc() since the function returns void * and that type is convertible to any object pointer type without a cast.
The total area might be close to 1018 units. For example if the input was

1
1 1000000000 1000000000

then your program would need to output

999999999000000000

and yet you are accumulating the area in the variable square, which is an int. Is int large enough on the target platform/compiler to store a number of this size? It certainly isn't on mine (x86-64/Clang) where INT_MAX is 2147483647.

This means that your program causes integer overflow, which yields undefined behaviour. (John Regehr has a good explanation here.)

On my platform (with item (1) above fixed) this bug causes mergebuilding() to run off the end of the array in an infinite loop (because the comparison with INF goes wrong) and this eventually hits unmapped memory and crashes with a segmentation fault.

I'm surprised that you didn't spot this at compilation time. I get these warnings from Clang:

cr26938.c:46:21: warning: implicit conversion from 'long' to 'int' changes value
      from 10000000000 to 1410065408 [-Wconstant-conversion]
    r[r_move].pos = INF;
                  ~ ^~~
cr26938.c:4:13: note: expanded from macro 'INF'
#define INF 10000000000
            ^~~~~~~~~~~

And these from gcc:

cr26938.c: In function ‘mergebuilding’:
cr26938.c:26: warning: comparison is always true due to limited range of data type
cr26938.c:26: warning: comparison is always true due to limited range of data type
cr26938.c:46: warning: overflow in implicit constant conversion

Perhaps you can say what compiler you are using?

You need to use a type that's large enough on the target platform to store numbers of the required size. If the target platform conforms to C99, then you can

#include <stdint.h>

and then use int64_t, which is guaranteed by the standard to be a signed integer type with exactly 64 bits. But if the target platform does not conform to C99, then, well, I guess you need to read the compiler manual to find out what type is big enough (but probably long or long long would work).
You write

#include <malloc.h>

which is non-standard: in Standard C, malloc() is declared in stdlib.h. Unless your platform is really non-standard, I'd stick with stdlib.h.
What is the role of the bul array? You only ever use one element at a time, and then only to read some numbers which you immediately copy out again. Why not read the input into variables on the stack instead? Perhaps like this:

for (int i = 0; i < num; i++) {
    int a, b, h;
    scanf("%d %d %d", &a, &b, &h);

You don't check the return values of the functions you call. malloc() returns NULL if it can't allocate, and scanf() returns the number of input items assigned, which might be less than the number of format specifiers if there was an error. I know this is only a simple program that doesn't require much in the way of error handling, but it's still an opportunity for you to practice your skills. And it doesn't have to be burdensome: you can write

if (p[i] == NULL) error("out of memory");
if (scanf("%d %d %d", &a, &b, &h) != 3) error("malformed input");

with error() defined like this:

void error(const char *message) {
    fprintf(stderr, "%s\n", message);
    abort();
}


Loop through array while creating one div.row and appending three div.col-lg-4 As a newbie to Javascript (or coding for that matter), I'm sure I overdid this, but I couldn't get anything else to work.  Here's what I'm doing:

allData in createDiv has anywhere from 20-500 elements ["string", "string",...] (created elsewhere)
I need it to create a div.row (I'm using bootstrap) and send to DOM
Append three elements from allData to that created div.row in DOM
Then repeat that cycle for allData.length

I couldn't figure out any other way to add one row and append three elements from the array to it.

createRow: function(number) {
    var parentDiv = document.getElementById("displayQuoteHere");
    var createRow = document.createElement("div");
        createRow.className = "row row-border";
        createRow.id = number;

    parentDiv.appendChild(createRow);
},
createDiv: function() {
    debugger;
    var allData = this.finalDellEqArray(); // allData = [100 or so elements]
    var incr = 0; // use this to go through my if else statements
    var parentRow = ''; // blank until first iteration - will be assigned according to [i]
    for (var i = 0; i < allData.length; i++) {
        if (incr === 0) {
            this.createRow(i); // run my createRow function and assign it an id (i) to use later in appendChild
            parentRow = document.getElementById(i);
            var createDiv = document.createElement("div");
                createDiv.className = "col-lg-4 text-center";  // using bootstrap to create three columns
                createDiv.innerHTML = allData[i]; // assign innerHTML to each element in array
            parentRow.appendChild(createDiv);
            incr++; // increase this to goto next else-if without creating a row
        } else if (incr === 1) {
            var createDiv2 = document.createElement("div");
                createDiv2.className = "col-lg-4 text-center"; //dito
                createDiv2.innerHTML = allData[i];
            parentRow.appendChild(createDiv2);
            incr++; // increase this to goto next else-if without creating a row
        } else {
            var createDiv3 = document.createElement("div");
                  createDiv3.className = "col-lg-4 text-center";
                createDiv3.innerHTML = allData[i];
            parentRow.appendChild(createDiv3);
            incr = 0; // set incr back to zero so it will start again and create a row.
        }
    }
},

I don't want to use any libraries as I'm trying to learn Javascript.  Suggestions or tips to make this better would be much appreciated!

Currently your code accesses live DOM unnecessarily, which is slow. For example, it creates a row, then searches it by id in the document instead of returning directly. It's better to construct elements without attaching them to actual document, and attach when everything's done.

However, since you're using innerHTML there's no need to create DOM elements manually, just construct an overall HTML string and insert it in the document. The entire code boils down to:

createDiv: function() {
  document.getElementById("displayQuoteHere").insertAdjacentHTML('beforeend',
    this.finalDellEqArray().map(function(item, index) {
      var before = index % 3 > 0 ? '' : '
';
      var after = index % 3 < 2 ? '' : '
';
      return before + '
' + item + '
' + after;
    }).join('')
  );
},


C binary number converter (Hex and Decimals) I'm still new to C programming, but I'm playing around with it to ensure I progress. I just made the following converter that takes 8 numbers (0 and 1) as binary code then converts the binary value to decimal and hex values. Below is the full code.

Aside from perhaps adding loops to make my code less redundant (especially the printf() statements), is there something I could do to make my code look more professional.

#include <stdio.h>
#include <math.h>

int main() {
int b1, b2, b3, b4, b5, b6, b7, b8, decimal, hex1, hex2;
char convert1, convert2;

printf("Please enter the 1st bit of your binary number (0 or 1): ");
scanf("%i", &b1);

printf("Please enter the 2nd bit of your binary number (0 or 1) ");
scanf("%i", &b2);

printf("Please enter the 3rd bit of your binary number (0 or 1) ");
scanf("%i", &b3);

printf("Please enter the 4th bit of your binary number (0 or 1) ");
scanf("%i", &b4);

printf("Please enter the 5th bit of your binary number (0 or 1) ");
scanf("%i", &b5);

printf("Please enter the 6th bit of your binary number (0 or 1) ");
scanf("%i", &b6);

printf("Please enter the 7th bit of your binary number (0 or 1) ");
scanf("%i", &b7);

printf("Please enter the 8th bit of your binary number (0 or 1) ");
scanf("%i", &b8);

decimal = b1 * pow(2,7) + b2 * pow(2,6) + b3 * pow(2,5) + b4 * pow(2,4) + b5 * pow(2,3) + b6 * pow(2,2) + b7 * pow(2,1) + b8 * pow(2,0);

hex1 = b1 * 8 + b2 * 4 + b3 * 2 + b4 * 1;
hex2 = b5 * 8 + b6 * 4 + b7 * 2 + b8 * 1;

switch(hex1) {
case 10:
    convert1 = 'A';
    break;
case 11:
    convert1 = 'B';
    break;
case 12:
    convert1 = 'C';
    break;
case 13:
    convert1 = 'D';
    break;
case 14:
    convert1 = 'E';
    break;
case 15:
    convert1 = 'F';
    break;
}

switch(hex2) {
case 10:
    convert2 = 'A';
    break;
case 11:
    convert2 = 'B';
    break;
case 12:
    convert2 = 'C';
    break;
case 13:
    convert2 = 'D';
    break;
case 14:
    convert2 = 'E';
    break;
case 15:
    convert2 = 'F';
    break;
}

printf("\nThe decimal value of your binary number is %i\n\n", decimal);
printf("The hexadecimal value of your binary number is: ");
if(hex1 >= 10 && hex2 < 10) {
    printf("%c%i\n", convert1, hex2);
}
else if (hex1 <10 && hex2 >= 10) {
    printf("%i%c\n", hex1, convert2);
}
else if (hex1 >= 10 && hex2 >=10) {
    printf("%c%c\n", convert1, convert2);
}
else {
    printf("%i%i\n", hex1, hex2);
}

return 0;
}

First things first, props to you for learning good old C in the day of dynamically typed high level languages! Now on to the review...

Error checking

Right now, there's nothing to stop me from entering 512351 as the seventh digit, for example. As a rule of thumb, if something comes from user input, double-check it. Your user is the greatest threat to program integrity. For example, check the input in a loop and only save it once it is a valid bit (0 or 1), if it's not valid, simply ask your user again. But this takes me to the next issue...

Input

Right now, you ask for each bit separately. I'm lazy. I wouldn't want to have 16 keystrokes to enter 8 digits. So, instead of asking for each bit on its own, consider just asking your user to type the digits as a string. Instead of eight integer variables, you could store them (temporarily, that is) in an array of characters. See the following snippet:

char input[8 + 1 + 1]; // 8 characters + newline + null terminator

printf("Enter a binary number: ");
fgets(input, sizeof input, stdin);
input[8] = '\0'; // We don't want the newline

This reads a line from standard input and stores the first 9 characters - 8 bits and a newline - in the array input, as well as a '\0' to terminate the string.

Now this is way shorter!

Conversion to decimal

But now that you have your digits as characters in a array, the conversion has to change. You can even be fancy and make your conversion more generic, in case you want to handle arbitrarily long numbers one day.

A typical idiom to get the numeric value from a character is int number = char - '0'; This subtracts the ASCII value for the character 0 from the character you have, leaving for example '5' as the integer 5. This will get strange when you use non-digit characters, of course. But you've checked that beforehand, I hope!

What's left is looping over your characters and adding their digit values to the total value.

int decimal_value = 0;

for (int i = 0; i != 8; ++i) {
    int digit = input[i] - '0';
    int factor = pow(2, 7 - i);

    decimal_value += digit * factor;
}

But since your binary digits are always either 0 or 1, you could tweak this a bit to prevent adding 0 a bunch of times.

int decimal_value = 0;

for (int i = 0; i != 8; ++i) {
    if (input[i] - '0') {
        decimal_value += pow(2, 7 - i);
    }
}

If the current character processed is '0', input[i] - '0' will evaluate to 0, which is falsy - the body of the if will not be executed. Otherwise, it is non-zero, which leads to the if-body being executed. Nice!

Conversion to hex

Right now, you do two things which should not be done together. You store the hex-digits as integer value, but once they're larger than 10 you store a character instead. This... works, but it's unusual and not that great. Instead, convert both two characters! The actual conversion is left as an exercise for you, but once you have both digits stored as first_hexdigit and last_hexdigit for example, you could to this:

char hex[2]; // Since we have two digits, let's store those as an array.
             // Note that, in contrast to before with input, we do not
             // have to make the array one larger than it needs to be,
             // as we do not use fgets which appends an extra character.

hex[0] = first_hexdigit >= 10 ? first_hexdigit - 10 + 'A'
                              : first_hexdigit + '0';
hex[1] = last_hexdigit >= 10 ? last_hexdigit - 10 + 'A'
                             : last_hexdigit + '0';

If you have never used it before, the ? and : are the ternary operator. Basically, a ? b : c means: if a is a truthy value, then return b, else c. Or, in other words:

int result;
if (a) {
    result = b;
} else {
    result = c;
}

To convert from a numeric value to a character, we switch our previous idiom around a bit: instead of subtracting our lowes character, we add it as value. If last_hexdigit is 6, for example, this becomes 6 + '0' which is just '6'. But if it's 12, the computation is 12 - 10 + 'A', which is 2 + 'A' which is 'C'.

Output

Since you've stored your results a bit differently, your way of printing has to be changed as well!

Assume we have our decimal value stored, as integer, in decimal_value and our hexadecimal as array of two characters in hexadecimal_value.

int decimal_value; // Your decimal
char hexadecimal_value[2]; // Your hexadecimal

printf("Decimal: %d\n", decimal_value);
printf("Hexadecimal: %c%c\n", hexadecimal_value[0], hexadecimal_value[1]);

This could made better a bit if you do make the hexadecimal_value array one larger than it needs to be and set the last character to '\0'. Because then, hexadecimal_value would be a string (as strings need to be null-terminated), which you could easily print with printf:

printf("Hexadecimal: %s\n", hexadecimal_value);

Encapsulation

Now that the actual code is cleaned up a bit, what can we do to make it more beautiful on a higher level?

Exctract functionality into functions, of course!

Right now, all your code is in main. This is okay for small programs like this, but should still be avoided. So instead, let's make some smaller functions for simple tasks! Your control flow is much like the following:

int main() {
    get_input();
    process_input();
    print_output();
}

Since it makes sense to handle the input and output in main (for this scale at least), let's focus on making the process_input()!
I suggest you use three functions, which carry your main logic:

int   is_valid_binary(const char *string);
char *binary_to_decimal(const char *string);
char *binary_to_hexadecimal(const char*string);

is_valid_binary takes in a string and returns 0 if it is not a valid binary number (e.g. contains characters other than '1' and '0'), otherwise, it returns 1. binary_to_decimal takes in a string of binary digits and returns a string of the very same number, but with decimal digits. binary_to_hexadecimal works similar; it takes a string and returns a string of hexadecimal digits. I leave actually writing these functions to you.

Once you have those three functions, your main would thus become:

int main() {
    char input[8 + 1 + 1];
    printf("Enter a binary number: ");

    do {
        fgets(input, sizeof input, stdin);
        input[8] = '\0';
    } while (!is_valid_binary(input));

    char *decimal = binary_to_decimal(input);
    char *hexadecimal = binary_to_hexadecimal(input);

    printf("Decimal: %s\nHexadecimal: %s\n", decimal, hexadecimal);

    // Since you allocated the memory for decimal end hexadecimal yourself,
    // you have to free it yourself as well! At least, you should allocate
    // it yourself.
    free(decimal);
    free(hexadecimal);
}

Exercise to you

To recap, consider the final version of main I pasted above. Your task now is to write the functions is_valid_binary, binary_to_decimal and binary_to_hexadecimal. You could also expand those functions to handle arbitrarily long binary numbers (not necessarily eight digits, but also 2 digits or 24 digits!). And, as an extra, take the input from the commandline. Use argv for that one.

Addendum

As Toby Speight mentioned in the comments, it is not really a good idea to use pow() when calculating 2^n. First of all, pow() uses doubles, which adds some conversion overhead (and, worst case scenario, wrong results due to the way double works. But it's rather unlikely.), and second: it's slow. Let me introduce you to bitmagic!

Consider the number 4, or 0b0100. 4 times 2 is 8, or 0b1000. There is a pattern going on here which is everywhere: if you multiply any number by 2, its binary representation gets shifted by 1 to the left. Since exponention is just repeated multiplication, 2^7 is just 1 * 2 * 2 * 2 * 2 * 2 * 2 * 2. That's seven left-shifts right there! In C, performing left-shifts is done by using the << operator.

So, instead of calculating 2^7 by using the slow pow(), you can use 1 << 7 instead!

Basically: pow(2,x) == 1 << x.


getfloat, the floating point analog of getint Write getfloat, the floating-point analog of getint. What types does getfloat return as its function value?

gefloat would also return an integer value.
Here is my solution:

int getfloat(double *pf) {
    int c, sign;
    double power;

    while(isspace(c = getch()))
        ;
    if(!isdigit(c) && c != EOF && c != '-' && c != '+' && c != '.') {
        ungetch(c); /* push the number back in buffer */
        return 0; /* not a valid number */
    }

    sign = (c == '-') ? -1 : 1;
    if(c == '-' || c == '+')
        c = getch();

    if(!isdigit(c) && c != '.') {
        ungetch(c);
        return 0; /* not a number */
    }

    for(*pf = 0.0; isdigit(c); c = getch())
        *pf = *pf * 10 + (c - '0');

    if(c == '.')
        c = getch();

    for(power = 1.0; isdigit(c); c = getch()) {
        *pf = *pf * 10 + (c - '0');
        power *= 10;
    }

    *pf *= sign;
    *pf /= power; /* moving the decimal point */

    if(c != EOF)
        ungetch(c);
    return c; /* it actually returns the value of the charachter that caused the break of the second for */
}

I have changed the type of the parameter *pf to double, thus it will handle the floating point represenation of a number. The condition of the first if-statement was extended, such that it will not push back in to the buffer a decimal point.
The second for computes the decimal part of the number, storing the number of places that the decimal point should pe moved in the variable power. If there is no decimal part, power will have the value 1 - thus, nothing will change if I devide *pf by power when power is 1.

It obviously does not handle NAN and INF.  Maybe outside code's goals.
Surprisingly does not handle exponential notation such as 1.23e+56.
if(!isdigit(c) && c != '.') { returns a 0 implying "not a number" and ungets the offending c.  But a potential + or - was all ready consumed and not available for alternative parsings.  Not sure best way to handle, but maybe consider that once a non-white-space char is irrevocable consumed, this function should not return a error signal and should set *pf to _something_like 0 or NAN.
Minor: The *pf /= power; approach fails with input such as 123.00000...(300+ zeros) as power or *pf already became INF.
This is a pedantic point: Once you get about 17 (depends on double) decimal digits, the addition of (c - '0') to *pf * 10 becomes moot.  To get a more accurate significand, forming the number right-to-left has precision advantages.
The return c; could return 0 indicating "not a number" .  Recommend an alternative return value scheme.
ungetch is not in the C spec.  Consider ungetc().
With ungetc() "If the value of c equals that of the macro EOF, the operation fails and the input stream is unchanged." thus negating the need for the if(c != EOF).  Still not bad to have there.
The decimal point . is locale sensitive and could be , or something else.  Use locale sensitive routines to determine the current decimal point.
You are commended for for taking in "-0" and rendering -0.0 rather than 0.0.
[Edit] IEEE_754-2008 Character_representation discusses the maximum number of non-zero leading digits to use when converting from text to a number.  double is often a IEEE_754 binary64, so a maximum number to use is 17+3.  This goes into point #3 above.  Its a bit deep on how this affects things, but I'll just note it for those who want to delve into a portable robust solution.


Displaying MySQL data using PHP I have MySQL data that I am rendering in an HTML table using PHP. This is the full code of my table and PHP. It does output fine and looks how I would expect but I am wondering if there is a better way to write this code. Also, the 'Delete' button specifically, is there a better way to output the button itself as right now, I noticed especially on a mobile device, I will see a text cursor indicator when tapping the button as if I am trying to select the text on the button itself.

                    Description

                    Date Created

                    Created By

                    Edit - Delete

        <?php

        $noLists = "
You have no lists. Why dont you create one?
";

        if ($db->connect_error) {
            die("Connection failed: " . $db->connect_error);
        }

        $sql = "SELECT DISTINCT lists.id, lists.listdescription, lists.createdon, lists.createdby, lists.sharedlist FROM lists INNER JOIN users ON users.fname = '$displayname' AND users.fname = lists.createdby";
        $result = $db->query($sql);

        if ($result->num_rows > 0) {
            // output data of each row
            while($row = $result->fetch_assoc()) {

                $listID = $row["id"];
                $listDescription = $row["listdescription"];
                $createdOn = $row["createdon"];
                $createdBy = $row["createdby"];

                echo "<form>";
                echo "
";
                echo "$listDescription	";
                echo "$createdOn	";
                echo "$createdBy	";
                echo "";
                echo "<form action='account.php' method='post'>";
                echo "<input class=\"btn btn-success\" type=\"submit\" name=\"\" value=\"Edit\">";

                echo "<input class=\"btn btn-danger\" value=Delete style=\"width: 85px;\">

                	";

            }
        } else {
            echo "
";
            echo "
";

            echo "
";
            echo "
";
            echo "
 $noLists
";
        }
        $db->close();

        ?>

Generally speaking, you should keep your markup and your PHP logic separate.

And the intermediate variables don't really have a purpose, just use the values from the array.

You've got two opening form tags and no closing form tags.

Your first condition (i there are records in the table) does not close the thead or the create a tbody.

Your delete button is no button at all, it's an input wrapped in an anchor tag (). That's why it's not working as expected. Either remove the anchor and give the input a type attribute of "button", or remove the input and put some text in there instead.

Use a prepared statement. It's not cool to throw variables into your queries all willy-nilly. You will anger the code gods.

Here's a quick re-write (assumes you're using PDO - If you're using MySQLi, see the docs to convert the prepared statement to MySQLi)

I'm just going to go ahead and post this since i spend three or four minutes on it, but after looking at this code, there is no way this was working as you had it. This site is for reviewing working code, not fixing broken code. If your code doesn't work you need to ask on StackOverflow in the future.

<?php

    // Do all the PHP stuff before you start ouputting
    $noLists = "
You have no lists. Why dont you create one?
";

    if ($db->connect_error) {
        die("Connection failed: " . $db->connect_error);
    }

    $sql = "SELECT DISTINCT lists.id, lists.listdescription, lists.createdon, lists.createdby, lists.sharedlist FROM lists INNER JOIN users ON users.fname = ? AND users.fname = lists.createdby";
    $result = $db->prepare($sql);
    $result->execute(array($displayname));
    $output = array();
    if ($result->rowCount() > 0) {
        // output data of each row

        $output[] = "
";
        $output[] = "
";

        while($row = $result->fetch(PDO::FETCH_ASSOC)) {

            $output[] = "
";
            $output[] = "{$row["listdescription"]}	";
            $output[] = "{$row["createdon"]}	";
            $output[] = "{$createdBy}	";
            $output[] = "";
            $output[] = "<form action='account.php' method='post'>";
            $output[] = "<input class=\"btn btn-success\" type=\"submit\" name=\"\" value=\"Edit\">";
            $output[] = "</form>";
            $output[] = "Delete	";
        }

        $output[] = "
";
        $output[] = "
";

    } else {
        $output[] = "
";
        $output[] = "
";
        $output[] = "
";
        $output[] = "
";
        $output[] = "
 $noLists
";
    }
    $db->close();

    // Turn the output array back into a string
    $output = implode('', $output);

?>

                    Description

                    Date Created

                    Created By

                    Edit - Delete

        <?php echo $output; ?>

Edit... you're not closing your table row in the loop either. I'm not gonna bother updating the code snippet.


Find the common elements of the lists in a list of lists Considering the problem:

Make a function to find the common elements of the lists in a list of lists.
Example:
common ( [ [1,2,3,3,5,11], [2,2,2,3,3,5], [2,2,3,3,4,5,6,7,18,19], [3,5,10,15] ] ) = [3, 5].

I came up with this solution:
import Data.List(group, nub, sort)

-- |
-- This function returns the elements common to all lists in a list of lists.
--
commonXss :: Ord a => [[a]] -> [a]
commonXss xss = concat [x | (x, y) <- countElemAppearsXss xss, y == length xss]

-- |
-- This function takes a list of lists and returns a list of tuples. Each tuple
-- (x, y) is made of x == [element] and y == number of lists in which that
-- element appeared.
--
countElemAppearsXss :: Ord a => [[a]] -> [([a], Int)]
countElemAppearsXss xss = freq $ sort $ concat $ map nub xss

-- |
-- This function returns a list of tuples. Each tuple (x, y) is made of
-- x == [element] and y == it's frequency.
--
freq :: Ord a => [a] -> [([a], Int)]
freq xs = map (\x -> ([head x], length x)) . group . sort $ xs

Is there a simpler way, more straightforward way that I'm missing? Perhaps in a library? How about the style?

This problem is just a matter of folding using the intersect function of Data.List, then deduplicating the result using nub.


Calculating the minimum and maximum of a byte array I've been working with binary files with this last assignment and part of it was to determine the maximum and minimum byte. Initially this was 2 functions but a bonus portion was to make them into 1 function through usage of an enum, I imagine to reinforce their utility. I would like to know if there are better ways of doing this, be it efficient, elegant or otherwise. I'd also like tips on my C code, as this is the first real project I've done in C in several years.

I've also included the relevant bits from the header file.

#define GOOD 1
#define BAD 0

typedef unsigned int uint;
typedef unsigned char byte;

typedef enum uMaxOrMin
{
    Max,
    Min
} MaxOrMin;

typedef enum uBitStyle
{
    BitsOn,
    BitsOff
} BitStyle;

int MaxOrMinValue(byte src[], uint size, byte *val, MaxOrMin mom)
{
    uint srcTemp;
    uint srcIndex = 0;
    uint largest;
    uint smallest;
    uint* data = src;

    switch (mom)
    {
    case Max:
    {
        largest = src[0]; //initialize largest
        while (*data++) //iterate over binary array
        {
            srcTemp = *(src + srcIndex++); //start at beginning of array, grab first byte and increment by 1
            if (srcTemp > largest) //current byte is greater than our saved largest byte
                largest = srcTemp; //set new largest
        }

        *val = largest; //return max to caller
        return GOOD;
    }

    case Min:
    {
        smallest = src[0]; //initialize smallest
        while (srcIndex < size) //iterate over binary array
        {
            srcTemp = *(src + srcIndex++); //start at beginning of array, grab first byte and increment by 1
            if (srcTemp < smallest) //current byte is less than our saved smallest byte
                smallest = srcTemp; //set new smallest
        }

        *val = smallest; //return min to caller
        return GOOD;
    }
    return BAD; //if we hit here, we've failed
    }
}

I know that under normal circumstances, C functions should return 0 upon success and -1 or otherwise upon failure, but this configuration was mandated by the assignment.

I would like to know why my while loop for BitsOn works but if I use the same construct for BitsOff, while (*data++), it fails to give me the correct results. Can someone explain this? Perhaps my understanding is lacking here. I'm trying to get more used to pointers so it seems pertinent to maximize my use of them, even in non-ideal circumstances.

Your combined function essentially looks like this:

// commonly used variables
if (min) {
    // code for computing the min
    return minValue;
} else if (max) {
    // code for computing the max
    return maxValue;
} else {
    return BAD;
}

Since there is no common code, you should either:

put the calculation for min and max in two separate functions
merge the code (which makes the code a little slower)

In the current code the variable definitions are too far away from the code that actually uses them. Understanding the code is easier when everything fits on a single screen.

Continuing with your code:

#define GOOD 1
#define BAD 0

These two constants may come from the 1980s or 1990s, when <stdbool.h> was not available yet. Nowadays functions that return success or failure should do so by returning bool instead of int.

typedef unsigned int uint;
typedef unsigned char byte;

typedef enum uMaxOrMin
{
    Max,
    Min
} MaxOrMin;

Since you don't use the uMaxOrMin name later, you can omit it.

typedef enum uBitStyle
{
    BitsOn,
    BitsOff
} BitStyle;

This whole enum is unused and should therefore be removed.

int MaxOrMinValue(byte src[], uint size, byte *val, MaxOrMin mom)

This function is not supposed to modify the contents of the src array. Therefore it should be declared as const byte src[].

To avoid the common confusion between * and [] for parameters (only the pointer is passed, not the whole array), it is preferable to write const byte *src instead.

The name of the size param is not as precise as possible. It should rather be src_size to make it unambiguous that it belongs to src.

{
    uint srcTemp;

The only time where temp is allowed as part of a variable name is in the popular pattern to swap two variables: tmp = a; a = b; b = tmp. In all other situations there must be a better name.

    uint srcIndex = 0;

Since there is only one thing that can be indexed here, the src in srcIndex is redundant. The code is short enough that the variable names can be short, too.

    uint largest;
    uint smallest;
    uint* data = src;

    switch (mom)
    {
    case Max:
    {
        largest = src[0]; //initialize largest

Accessing src[0] is only allowed if src_size > 0.

        while (*data++) //iterate over binary array
        {
            srcTemp = *(src + srcIndex++); //start at beginning of array, grab first byte and increment by 1

There is no point in writing *(ptr + index) since ptr[index] is much clearar to read.

            if (srcTemp > largest) //current byte is greater than our saved largest byte

Ah, so srcTemp really means "the current byte". In that case, the variable should be named curr.

                largest = srcTemp; //set new largest
        }

        *val = largest; //return max to caller

In this assignment, you are storing an unsigned int into a byte variable. Because both are unsigned, this works. Better style would be to make largest a byte variable, too. In other programming languages like Go or Kotlin, assigning values between different-sized variables is a compile-time error, and I find these quite helpful.

        return GOOD;
    }

    case Min:
    {
        smallest = src[0]; //initialize smallest
        while (srcIndex < size) //iterate over binary array

Since min and max are essentially the same algorithms, their code should express this by being essentially the same. You are using two entirely different loops here, which leads to confusion and bugs.

        {
            srcTemp = *(src + srcIndex++); //start at beginning of array, grab first byte and increment by 1
            if (srcTemp < smallest) //current byte is less than our saved smallest byte
                smallest = srcTemp; //set new smallest
        }

        *val = smallest; //return min to caller
        return GOOD;
    }
    return BAD; //if we hit here, we've failed
    }
}

All in all, your code does not take any advantage of the algorithms being so similar. To combine them in a single piece of code, I suggest this:

#include <assert.h>

#define GOOD 1
#define BAD 0

typedef unsigned int uint;
typedef unsigned char byte;

typedef enum {
    Max,
    Min
} MaxOrMin;

int MaxOrMinValue(const byte *src, uint src_size, byte *result, MaxOrMin mom) {
    if (src_size == 0) {
        return BAD;
    }
    if (mom != Min && mom != Max) {
        return BAD;
    }

    byte mask = (byte) (mom == Max ? -1 : 0);
    byte best = src[0] ^ mask;
    for (uint i = 1; i < src_size; i++) {
        byte curr = src[i] ^ mask;
        if (curr < best) {
            best = curr;
            if (best == 0)
                break;
        }
    }

    *result = best ^ mask;
    return GOOD;
}

int main() {
    byte arr1[] = {0, 1, 2, 3, 4};
    byte arr2[] = {200, 100, 222};
    byte minmax = 13;

    assert(MaxOrMinValue(arr1, sizeof arr1, &minmax, Min) == GOOD);
    assert(minmax == 0);

    assert(MaxOrMinValue(arr1, sizeof arr1, &minmax, Max) == GOOD);
    assert(minmax == 4);

    assert(MaxOrMinValue(arr2, 0, &minmax, Max) == BAD);
    assert(minmax == 4); // unchanged

    assert(MaxOrMinValue(arr2, 1, &minmax, Max) == GOOD);
    assert(minmax == 200);

    minmax = 123; // to make sure it is overwritten again
    assert(MaxOrMinValue(arr2, 2, &minmax, Max) == GOOD);
    assert(minmax == 200);

    assert(MaxOrMinValue(arr2, 3, &minmax, Max) == GOOD);
    assert(minmax == 222);

    // Passing neither Min nor Max is bad.
    assert(MaxOrMinValue(arr2, 3, &minmax, 123) == BAD);
    assert(minmax == 222); // unchanged
}

Admitted, using a bitmask to combine the min and max algorithms is tricky. Without using this trick, the code might look like this:

int MaxOrMinValue(const byte *src, uint src_size, byte *result, MaxOrMin mom) {
    if (src_size == 0) {
        return BAD;
    }
    if (mom != Min && mom != Max) {
        return BAD;
    }

    byte best = src[0];
    byte best_possible = mom == Max ? (byte) -1 : (byte) 0;
    for (uint i = 1; i < src_size; i++) {
        byte curr = src[i];
        if (mom == Max ? curr > best : curr < best) {
            best = curr;
            if (best == best_possible) {
                break;
            }
        }
    }

    *result = best;
    return GOOD;
}

Oh, nice. The code even got simpler. One thing that bothered me though about this code is the complicated condition in the if (mom == Max ?. I wanted to avoid checking this condition in every loop again because of branch prediction and possible performance.

During one call of this function, the outcome of the outer condition will always be the same though (mom does not change at all). Therefore the actual performance penalty might be so small that it is barely measurable. Choosing the faster code would need very careful benchmarks in this case. Therefore, either variant is probably good enough.

Anyway, out of intuition I wanted to avoid as many branches as possible. To avoid the mom == Max condition inside the loop, I quickly analyzed in my head that min == bitinvert(max), at least for unsigned numbers. And since you defined the byte type as unsigned char, it seemed a good fit. Plus, I wanted to try out this bitmasking idea since I had not seen it anywhere else before.

Finding the idea of using a bitmask to convert between min and max algorithms is something that probably comes with experience, I would be very surprised if any programming beginner would find this by themselves. Even understanding it is difficult enough. For more crazy stuff, have a look at the book Hacker's Delight.

One nice last thought about this bitmasking technique is that I think it can be extended even further to also allow MinSigned (mask 0b1000_0000) and MaxSigned (mask 0b0111_1111). There might also be even stranger use cases like mask 0b0011_0000 to find the smallest ASCII digit.

Things that the bitmasking technique cannot do are:

find the first uppercase letter from the alphabet (that is, 'A' if it exists somewhere, otherwise 'B', otherwise 'C'). This is because there are 26 uppercase letters, and the code point of the 'A' is U+0041, which in binary is 0100_0001. Using this as the bitmask, the smallest bytes would be 'A@CBEDHG', in this order.
find the largest ASCII digit, for similar reasons.


Is the solution design optimal? C# 3 - Tier I have the following solution structure. This is a business domain (for an amount transfer operation in bank account) which will be called by a WCF service. Is the solution structuring correct?

Data Acess Layer does not create domain objects. It just pass database record wrapped in another simple object (DataEntities.AccountRow). Is it a good/standard approach?
Manager and domain objects are in two different layers. Is it okay?
A layer “DTOforServiceCommunication” is created for communicating with WCF service.
Is DTOforServiceCommunication and DataEntities are redundant or a good practice?
What are the improvement points for this solution structure?

Note: The service mentioned above will be used by multiple business functions (clients). Since the SOA is not object oriented, we cannot pass business domain objects across the boundary.

// TransferDTO

namespace DTOforServiceCommunication
{
public class TransferDTO
{
    public int UserID { get; set; }
    public int FromAccounutNumber { get; set; }
    public int ToAccountNumber { get; set; }
    public int AmountToTransfer { get; set; }
}
}

// AccountRow

namespace DataEntities
{
public class AccountRow
{
    public int AccountNumber { get; set; }
    public string AccountType { get; set; }
    public int Duration { get; set; }
    public int DepositedAmount { get; set; }

}
}

// AccountManager

namespace BusinessManager
{
public class AccountManager
{
    public void TransferAmount(DTOforServiceCommunication.TransferDTO  transferDTO)
    {
        //DAL does not create domain objects. It just pass database record wrapped in another simple object
        DAL.AccountDAL accountDAL = new DAL.AccountDAL();
        DataEntities.AccountRow row = accountDAL.GetAcocunt(transferDTO.UserID, transferDTO.FromAccounutNumber);

        DomainObject.IBankAccount bankAccount = null;
        if (String.Equals(row.AccountType, "Savings"))
        {
            bankAccount = new DomainObject.SavingsAccount();
            bankAccount.UserID = transferDTO.UserID;
            bankAccount.AccountNumber = row.AccountNumber;
            bankAccount.AmountDeposited = row.DepositedAmount;

        }
        else
        {
                bankAccount = new DomainObject.FixedAccount();
                bankAccount.UserID = transferDTO.UserID;
                bankAccount.AccountNumber = row.AccountNumber;
                bankAccount.AmountDeposited = row.DepositedAmount;
        }

        bankAccount.Transfer(transferDTO.ToAccountNumber, transferDTO.AmountToTransfer);

    }

}
}

//DAL

namespace DAL
{
//DAL does not create domain objects. It just pass database record wrapped in another simple object
public class AccountDAL
{

    List<DataEntities.AccountRow> dbRecords = new List<DataEntities.AccountRow>()
    {
        new DataEntities.AccountRow{AccountNumber=1,AccountType="Savings",Duration=6,DepositedAmount=50000},
        new DataEntities.AccountRow{AccountNumber=2,AccountType="Fixed",Duration=6,DepositedAmount=50000}
    };

    public DataEntities.AccountRow GetAcocunt(int userID, int accountNumber)
    {
        return dbRecords[0];
    }

}

    }

READING:

http://msdn.microsoft.com/en-us/magazine/dd569757.aspx
https://stackoverflow.com/questions/125453/implementation-example-for-repository-pattern-with-linq-to-sql-and-c-sharp
https://stackoverflow.com/questions/3175/repository-pattern-tutorial-in-c-sharp/1374420#1374420
https://stackoverflow.com/questions/151769/whats-the-common-way-for-oop-pattern-design-data-access
https://stackoverflow.com/questions/4198136/classic-ado-net-or-entity-framework-better-for-larger-scale-database-transaction
http://sourceforge.net/projects/vanilla-dal/
https://stackoverflow.com/questions/9474425/should-the-configuration-of-an-application-be-accessed-using-dal

Data Acess Layer does not create domain objects. It just pass database record wrapped in another simple object (DataEntities.AccountRow). Is it a good/standard approach?

Repository classes should create domain objects. The domain objects may or may not look the same as the DB entities.

If you mean that you write your DAL by yourself (and not using a ORM): Stop with that. It's a waste of time.

  Manager and domain objects are in two different layers. Is it okay?

Use the term WCF service and not manager. Yes. I consider WCF services to be an UI layer since it's the interface to the calling user/client.

  A layer “DTOforServiceCommunication” is created for communicating with WCF service.

The name doesn't matter as long as you understand the difference between a domain object and a DTO.

  Is DTOforServiceCommunication and DataEntities are redundant or a good practice?

Good practice. The DTO's should never change since it would break all clients that use them.

  What are the improvement points for this solution structure?

No need for a userId in the WCF interfaces or the DTOs unless you want to let everyone be able to look at everybody elses accounts. Use the userid provided during authentication.

Update

Repository class using vanilla ADO.NET:

public class AccountRepository
{
    Dictionary<string, Type> _accountClasses = new Dictionary<string, Type>{{"savings", typeof(SavingsAccount)}, {"fixed", typeof(FixedAccount)}};
    List<DataEntities.AccountRow> dbRecords = new List<DataEntities.AccountRow>()
    {
        new DataEntities.AccountRow{AccountNumber=1,AccountType="Savings",Duration=6,DepositedAmount=50000},
        new DataEntities.AccountRow{AccountNumber=2,AccountType="Fixed",Duration=6,DepositedAmount=50000}
    };

    public T Get<T>(int userID, int accountNumber) where T : Account
    {
        var sql = "blabla";
        using (var cmd = _connection.CreateCommand())
        {
            cmd.CommandText = sql;
            using (var reader = cmd.ExecuteReader())
            {
                if (!reader.Read())
                    return null;

                var account = CreateRow(reader);
                if (account.GetType() != typeof(T));
                    throw new InvalidOperationException("The requested account was not of the specified type");

                return account;
            }
        }
    }

    public IEnumerable<Account> FindMyAccounts(int userId)
    {
        var sql = "blabla";
        using (var cmd = _connection.CreateCommand())
        {
            cmd.CommandText = sql;
            using (var reader = cmd.ExecuteReader())
            {
                List<Account> accounts = new List<Account>();
                while (reader.Read())
                {
                    var account = CreateRow(reader);
                    accounts.Add(account);
                }

                return accounts;
            }
        }
    }

    // IDataRecord = a row in a DataReader
    public Account CreateRow(IDataRecord record)
    {
        // might want to check so that the account got a
        Type type;
        if (!_accountClasses.TryGetValue(record["AccountType"], out type))
            throw new InvalidOperationExcpetion("Account type do not exist: " + record["AccountType"]);

        var account = (Account)Activator.CreateInstance(record);

        // fill record here
    }

}

public class Account
{
}

public class SavingsAccount : Account
{
}

public class FixedAccount : Account
{
}

Usage:

account = _repository.Get<SavingsAccount>(1, "kdkdkdkdkd");

Update2

Added a Find method to show that the actual mapping is reused by all methods.

The Find method will also return different types of accounts in the same list.

You could also break out the ExecuteReader parts into two methods (one to fetch one item and one to fetch collections) to reuse the code more.


Insert a character into a string For practicing reasons I would like to write a method which inserts a character into a string.

I would like to know:

What is the best practice concerning placement of a comment within methods?  For example, is the way that I commented about what raises an exception fine, or are there better practices?  Would I need more comments, and if so, where?
In this particular example, are throwing unchecked exceptions preferred or checked?
How can I make this method more reliable?
How can I make this method more scalable?
While dealing with immutable strings there are always discussions around performance. Can I make this method run faster?
Is this code thread safe?

/**
 * Inserts the character ch at the location index of string st
 * @param st
 * @param ch
 * @param index
 * @return a new string
 */
    public static String insertCharAt(String st, char ch, int index){
        //1 Exception if st == null
        //2 Exception if index<0 || index>st.length()

        if (st == null){
            throw new NullPointerException("Null string!");
        }

        if (index < 0 || index > st.length())
        {
            throw new IndexOutOfBoundsException("Try to insert at negative location or outside of string");
        }
        return st.substring(0, index)+ch+st.substring(index, st.length());
    }

The Javadoc style headers you have provided for this method are already more than enough. If you are adding comments to your method it should only be either for API documentation, or where the code is sufficiently complex to warrant an explanation that you cannot provide by simplifying or more appropriately naming the variables.

You should however make it clear that your method throws these two exceptions by including them in the method definition, e.g.

public static String insertCharAt(String str, char ch, int index) throws NullPointerException, IndexOutOfBoundsException

This way it will be obvious from reading the method definition that these exceptions can be thrown, and your comment will be unnecessary when we read the line above each exception-throwing statement and see the logic that causes it to be thrown.

In general, if your comment is telling us nothing more than what the code already does, make your code as simple and readable as possible and delete the comment. At best it will add noise to the code file and at worst it will get out of sync with the code and become misleading.
In this example, presumably prefer to use a checked exception so that the method calling insertCharAt and can catch these exceptions and handle them in some way. See also https://stackoverflow.com/questions/27578/when-to-choose-checked-and-unchecked-exceptions
This depends what you mean by more reliable. Do you mean robust in regards error cases?
This also depends on what you mean by more scalable. What do you mean?
This method is very simple and there isn't a great deal of room for performance improvement that I can see.

Edit:
This method is thread safe as it does not modify any parameters passed to it (and strings are immutable anyway) and does not modify any class-local variables. Even if you had assigned the return value to a temporary variable before returning it, it would not be shared between threads as method level variables belong to each thread's stack.


Speed up javascript code to convert csv to geojson I'm using a script that converts a CSV file into GeoJSON in order to display it on a map. You can see it for yourself here. The CSV has about four thousand rows. It's quite slow. I'm wondering what is slowing it down and how I can speed it up.
function latLonColumnsToNumbers(data, latName, lonName) {
  return data.map(function(item) {
    if (item.hasOwnProperty(latName)) {
         item[latName] = parseFloat(item[latName]);
         if (isNaN(item[latName])) { //check if row has blank lat/long, and if so, flag it
            var latFlag = 1;
         } else {
             latFlag = 0;
         }
      }
    if (item.hasOwnProperty(lonName)) {
         item[lonName] = parseFloat(item[lonName]);
         if (isNaN(item[lonName])) {
             var lonFlag = 1;
         } else {
             lonFlag = 0;
         }
    }
    if (latFlag == 1 || lonFlag == 1) { //if row has no lat/long, then assign it to the approximate center of the map
        item[latName] = 42.2;
        item[lonName] = ~83.6;
        return item;
     }
    else {
        return item;
    }
  });

I know that a faster way to do this would be to turn the CSV into a sql database and call it with PHP but that involves using a lot of server-side stuff like NodeJS, Ajax, etc that I am not (yet) familiar with. So I would like to keep using the CSV for now until I learn how to do that.

From a short review;

Old school for loops still beat map

Prevent repeated object[string] access, like accesssing several times item[latName], because it is relatively slow

Is it absolutely necessary to create a new object? I have a suspicion that modifying the object would be faster. (Yes, not great practice, but you are looking for speed, you should try this)

One more thing, I was hesitant to mention this but, but the plus operator can be faster than parseFloat according to jsperf. So you could just go for
  const newLat = +item[latName];
  const newLong = +item[longName];

give it a shot, let us know ;)
This is my counter proposal with in place conversion;
function convertLocationToNumbers(data, latName, longName) {
  const dataLength = data.length;
  for(let i = 0; i < dataLength; i++){
    const item = data[i];
    //Dont check for hasownproperty, just go!
    const newLat = +item[latName];
    const newLong = +item[longName];
    if(isNaN(newLat) || isNaN(newLong ){
      item[latName] = 42.2;
      item[lonName] = ~83.6;
    }else{
      item[latName] = newLat;
      item[lonName] = newLong;
    }
  }
}


Check if a given string matches a wildcard pattern I'm trying to optimize my solution to the following question:

given a regular expression with chars and special char '*', (star which is a joker we can replace with any string), and a string, write a program which determines if the regular expression can represent that string or not.
for example: "a'*b" can represent "aaaacbbbb" , in contrast "a'*b'*c" cant represent "aaaabcccccccccb"

I solved the problem and inserted the code below, I want to optimize either space or time, and if possible to make it more readable or shorter.
#include <stdio.h>
#include <string.h>

int compareHelper(char *s, char *regex, int star);

int compareStrings(char *s, char *r);

int main()
{
    printf("%d\n", compareStrings("aaaacbbb", "a*b"));
    printf("%d\n", compareStrings("aaaabccccccccb", "a*b*c"));
    return 0;

}

//aaaaacbbb
//a*b

int compareStrings(char *s, char *r)
{
    return compareHelper(s, r, 0);

}

int compareHelper(char *s, char *regex, int star)
{
    if (*s == '\0' && *regex == '\0')
    {
        return 1;
    } else if (*regex == '*')
    {
        return compareHelper(s, regex + 1, 1); // star is seen hence will be set to true.
    } else
    {
        if (!star)
        {
            if (*s != *regex)
            { // star was not seen and they differ in char
                return 0;
            } else
            {
                return compareHelper(s + 1, regex + 1, star);
            }

        } else
        {
            while (*s && *s != *regex)
            {
                s++;
            }
            if (*s == '\0') return 0;
            else
            {
                return compareHelper(s + 1, regex + 1, 0) | compareHelper(s + 1, regex, 1);
            }
        }
    }
}

The code you wrote can be improved in lots of places. I'll go through it from top to bottom.
#include <stdio.h>
#include <string.h>

Perfect until now. The #include lines from the C standard library should be in alphabetic order.  I cannot say whether you did this intentionally or not since there's a 50-50 chance. :)
int compareHelper(char *s, char *regex, int star);

This function should be declared as static, since it is local to this file. To do this, replace the int compareHelper with static int compareHelper.
The compare in the function name is wrong. Typically, to compare two things, they have to be of the same or at least a compatible type. The technical type here is char *, but on a more abstract level, s is an arbitrary string while regex is a string with some constraints. Instead of compare, this function should be called match.
The type of the string parameters should be const char * instead of char * since this function does not modify the characters of these strings.
The type of the parameter star should be bool instead of int. To get this type, you have to add #include <stdbool.h> along with the other #include lines.
The name of the parameter regex is wrong. The word regex means regular expression, and this is a fixed term with a precise definition. Your patterns are not regular expressions, therefore the parameter name confuses the reader. Better call it pattern.
int compareStrings(char *s, char *r);

Again, the compare should rather be match, the parameter types should get an additional const, and the return type should be bool instead of int.
int main()
{
    printf("%d\n", compareStrings("aaaacbbb", "a*b"));
    printf("%d\n", compareStrings("aaaabccccccccb", "a*b*c"));
    return 0;
}

The empty parentheses () in the function declaration of main mean that the function can take any number of parameters. In C, this is only allowed for legacy code from 1990 and earlier. In modern code, write (void) instead to clearly say that there must be no parameters.
//aaaaacbbb
//a*b

This is the beginning of documentation, which is a good idea. To be useful for other readers, you should write in whole sentences though, for example:

// See if the string s matches the pattern p.
// In the pattern, '*' matches an arbitrarily long sequence of arbitrary characters.

int compareHelper(char *s, char *regex, int star)
{
    if (*s == '\0' && *regex == '\0')
    {
        return 1;
    } else if (*regex == '*')

After the return 1, the else is unnecessary. I would write the code like this instead:

    if (*s == '\0' && *regex == '\0') {
        return 1;
    }

    if (*regex == '*') {
        ...

This way, your code has one topic per paragraph. The empty line between the topics clearly marks a pointer where the reader may stop reading and take a deep breath, before starting to read the next paragraph.
The rest of the code looks fine, except for this line:
return compareHelper(s + 1, regex + 1, 0) | compareHelper(s + 1, regex, 1);

The operator | means a logical or, and it always evaluates both operands. This means that compareHelper is called twice, even if the left-hand side already evaluates to true. This is not necessary. To only evaluate the necessary parts of the condition, replace the | operator with ||.
All in all, the code is not too long. What makes it look verbose is that you put each { or } on its own line. You can rewrite it like this:
#include <stdbool.h>

int compareHelper(const char *s, const char *regex, bool star)
{
    if (*s == '\0' && *regex == '\0')
        return true;

    if (*regex == '*')
        return compareHelper(s, regex + 1, 1); // star is seen hence will be set to true.

    if (!star) {
        if (*s != *regex)
            return false; // star was not seen and they differ in char
        else
            return compareHelper(s + 1, regex + 1, star);
    }

    // seen a star; look for the text after the star
    while (*s && *s != *regex)
        s++;

    if (*s == '\0')
        return 0;

    return compareHelper(s + 1, regex + 1, 0) || compareHelper(s + 1, regex, 1);
}

Some people will complain that "you must always use braces in if and while statements", but that's not necessary. If you properly indent your code or let your IDE format the code automatically, the indentation properly expresses the relation of the code pieces to each other. The typical example is:
if (condition)
    goto fail;
    goto fail;

Modern compilers not only translate the source code to machine code, they also check for various possible bugs in the code. Wrong indentation like in the above snippet is one of them.
And here's a complete implementation that also matches character ranges like [A-Z]. The code looks remarkably similar to yours, which is good. If you take that code and remove the part that handles the [A-Z] part, it will be very similar to your code. Actually, taking other people's code and editing it just trying to understand it is an efficient way of thinking about why each of these lines of code is actually necessary.
While reading that code, you may notice that your code might fail for str_match("a good game", "a *game"). You should verify that the search after the star looks at every possible position in the string, not only the first time the letter g appears.


Laundry - sort socks This is very artificial problem
Each sock has exactly 1 match.
Can only take one random sock out of the Laundry basket.
Can only compare to the last random sock.
If it matches you put the match aside.
It is like you have a stack of socks and you can only match on the top sock.
So if the match is down the sock pile you are not going to find a match.
But you don't even get to look down the sock pile to even know if the match is in the sock pile. You only get to match to the top sock.
You will eventually need to put the sock pile back in the Laundry basket.
You can throw the unmatched socks back in the Landry basket at any time but they come out random

On CS there is a guy telling me he has a better algorithm but he cheats and searches the sock pile for a match down the pile.

public static void MatchSocks()
{
    Random rand = new Random();
    List<int> Laundry = new List<int>();
    List<int> Matched = new List<int>();
    List<int> Unmatched = new List<int>();
    int? LastUnmatched = null;
    int Sock;
    int count = 0;
    for (int i = 0; i < 500; i++)
    {
        Laundry.Add(i);
        Laundry.Add(i);
    }
    while (true)
    {
        count++;
        if(Laundry.Count == 0)
        {
            if (Unmatched.Count == 0)
                break;
            Laundry = new List<int>(Unmatched);
            Unmatched.Clear();
            LastUnmatched = null;
        }
        Sock = Laundry.ElementAt(rand.Next(Laundry.Count));
        Laundry.Remove(Sock);
        if (LastUnmatched == null)
        {
            Unmatched.Add(Sock);
        }
        else
        {
            if (Sock == LastUnmatched)
            {
                Matched.Add(Sock);
                Unmatched.Remove(Sock);
            }
            else
            {
                Unmatched.Add(Sock);
            }
        }
        LastUnmatched = Sock;
    }
    Debug.WriteLine(count);
}

You're using some poor methods when there are better options available to the data types you've picked.

A huge bottleneck is hidden in those 2 lines:

Sock = Laundry.ElementAt(rand.Next(Laundry.Count));
Laundry.Remove(Sock);

Let's see why

General vs specific methods

ElementAt is an extension method of the type IEnumerable<T>, the List<T> is inheriting IEnumerable<T> but guess what, so are most of the collections in C#. That means when you write a method that is applicable to so many types, you need to make it as generic/general as possible while still maintaining reasonable performance, but of course using a concrete implementation of that method for said collection would be faster. You can do that because you are 100% sure that the optimizations will always be valid.

So the first thing is to replace the ElementAt() with normal indexer.

RemoveAt() vs Remove()

In most cases Remove() would be a O(n) operation where RemoveAt() would be O(1) operation. The first uses the latter internally so it's always better to use this method when you know what index you want to remove, guess what, you already know what index you're removing:

int index = rand.Next(Laundry.Count);
Sock = Laundry[index];
Laundry.RemoveAt(index);

Besides that you're never utilizing the collection Matched. You're just wasting time adding stuff into it without ever touching them.

Adding those changes made the code about 2.5 times faster. On average from 45ms  to about 17ms per run.


increase efficiency of loops and element-wise operations in PyTorch implementation For any input symmetric matrix with zero diagonals W, I have the following implementation in PyTorch. I was wondering if the following can be improved in terms of efficiency,
P.S. Would current implementation break backpropagation?
import torch

W = torch.tensor([[0,1,0,0,0,0,0,0,0],
                  [1,0,1,0,0,1,0,0,0],
                  [0,1,0,3,0,0,0,0,0],
                  [0,0,3,0,1,0,0,0,0],
                  [0,0,0,1,0,1,1,0,0],
                  [0,1,0,0,1,0,0,0,0],
                  [0,0,0,0,1,0,0,1,0],
                  [0,0,0,0,0,0,1,0,1],
                  [0,0,0,0,0,0,0,1,0]])

n = len(W)
C = torch.empty(n, n)
I = torch.eye(n)
for i in range(n):
    for j in range(n):
        B = W.clone()
        B[i, j] = 0
        B[j, i] = 0

        tmp = torch.inverse(n * I - B)

        C[i, j] = tmp[i, j]

A few hints for improving efficiency:

When W[i, j] == 0, B equals to W so tmp remains unchanged. In this case, the computation of C values can be done only once outside the loop instead of inside the loop.

torch.nonzero / torch.Tensor.nonzero can be used to obtain all indices of non-zero values in a tensor.

Since W is symmetric, C is also symmetric and only half of its values need to be computed.

All repeated computation can be moved out of the loop to improve effciency.

Improved code:
n = len(W)
nIW = n * torch.eye(n) - W
nIB = nIW.clone()
C = nIB.inverse()

for i, j in W.nonzero():
    if i < j:
        nIB[i, j] = nIB[j, i] = 0
        C[j, i] = C[i, j] = nIB.inverse()[i, j]
        nIB[i, j] = nIB[j, i] = nIW[i, j]

Further performance improvement may be achieved according to this.
As for backpropagation, I have no idea how it can be done on elements of matrix inverses.