Data-pulling fuctions The first function, sqlPull(), connects to a local MySQL database and pulls the last 20 rows from the database every 5 seconds.  The data coming in is a list of tuples, where MAC, RSSI and TimeStamp are the 3 elements in each tuple.  The data is entering the database at a variable rate, where sometimes I'll have tons of entries in a minute and sometimes I will have very few.  The data that is coming in consists of a MAC address, an RSSI and a TimeStamp for every time a user's device pings our hardware.  So, we will have unique MAC addresses that will have changing RSSIs and TimeStamps over a period of time.

Since I never know how many database entries will be added within the 5 seconds that the first function runs on, the second function, dupCatch() is used to detect and eliminate duplicates.

The third function, post(), just sends a POST request to Google Analytics with the data stripped of duplicates.

Here is how I would like to optimize my code:

My next step in this process is to calculate the length of time between a users first and last entries in the database.  We can assume that the max time between a user's entry and exit is 2 hours (in this time, the users would probably have 10-15 pings with different RSSIs and timestamps).  I have been trying to think of the best way to do this with the current code, but everything I am thinking of seems so unwieldy.  In my mind, sqlPull() and/or dupCatch() could be rewritten/optimized to better serve my purposes for a function to calculate length of time between entry and exit.

How would you pull data that is coming in at a variable and unpredictable rate?

import mysql.connector
import datetime
import requests
import time

run = True

def sqlPull():
    connection = mysql.connector.connect(user='XXXXX', password='XXXXX', host='XXXXX', database='XXXXX')
    cursor = connection.cursor()
    cursor.execute("SELECT TimeStamp, MAC, RSSI FROM wifiscan ORDER BY TimeStamp DESC LIMIT 20;")
    data = cursor.fetchall()
    connection.close()
    time.sleep(5)
    return data

seen = set()
def dupCatch():
    data = sqlPull()
    new_data = []
    for (TimeStamp, MAC, RSSI) in data:
        if (TimeStamp, MAC, RSSI) not in seen:
            seen.add((TimeStamp, MAC, RSSI))
            new_data.append((TimeStamp, MAC, RSSI))
    print new_data
    return new_data

def post():
    new_data = dupCatch()
    for ((TimeStamp, MAC, RSSI)) in new_data:
        payload = {'v':'1','tid':'XXXXXX','cid': '1','t':'event', 'ec': MAC, 'ea': 'InStore', 'el': RSSI}
        requests.post("http://www.google-analytics.com/collect", data = payload)

while run is True:
    post()

You need rate-limit the amount of records you receive. You'll have to determine what the best rate is by yourself, as that is specific to your scenario.

High Priority:

Keep the list of "synced" entries in the database. Something like an IsSentToAnalytics column.
After you process a batch, update the database records you just selected with the synced set to yes, or 1, or true. The simplest way to do this would be to update each entry one by one, but there are more efficient solutions to that out there.
Your database query should only select from items that are not yet synced. This will be lighter on the database. (i.e. add the predicate and where IsSentToAnalytics <> 1)
Your dupCheck is catering for the above flag, essentially you're pulling unused data every time if it's a quiet period hence why you need the dupCheck. And also, MISSING data if it's a busy period. That can happen if you got more than 20 entries added to your database since your last run. This may be fine for hobby projects, but shoddy for production deployments.
Please, please add a surrogate primary key to your database. If anything, it keeps you from having to generate a hash every time you want to do a dupCheck.
The above entry kind of depends on how you generate the data. But since you have timestamp in there, I'm assuming every single record is unique anyways, so you might as well use the surrogate key as suggested. By the way, the surrogate key will be a sequence used as a primary key. Every entry inserted gets a unique value.
Edit. From your feedback below, you say you sometimes have duplicate values for the same timestamp. If that's the case, then you need to filter them out before inserting into the database initially. Whatever job/service that creates those entries needs to filter out the duplicates. You can try insert, and catch the DBException because it will fail the column constraint. Or, you could first do a select, and if no result is returned, insert, otherwise discard. You'll need to edit your question with more info for us to give you more detail on that.

Low Priority:

Move your sleep method into your while loop, not in your SQL pull, please. There is no need to sleep before returning data. Put the sleep where it belongs. Either that or change your function to be called PullDataAndSleep. If you think that function name is fine, then don't change the code inside it, just rename. Otherwise, put the sleep elsewhere.
Reuse your connection. You are disconnecting and reconnecting to the database every 5 seconds, it's pointless unless you expect your database connection to drop.
Change your while loop to just be "while Run", that's like saying while var == True. You're basically saying if True == True, then run. It's redundant.
Pull from the database at constant intervals(you are using 5s, play around with it).
Enforce an upper-bound amount of returned entries (you have 20)
The last two entries above will help you rate-limit how many requests you send off to Google. If you spam them too quickly, they might block/ban your IP.


Excel-like column numbering I have a working code (below) to convert a number to Excel-like (A...Z AA...AZ BA...BZ ...ZA-ZZ) column codes (headers).

For simplicity I limited it to 1 or 2 character codes.

Can the algorithm be streamlined by using a bit more of elementary math or using it differently?

Please note that I just happened to need it in JavaScript (so I wrote it in JS) and am not looking for pure code improvements.

function numberToLetters(nNum) {
    var result;
    if (nNum <= 26) {
        result = letter(nNum);
    } else {
        var modulo = nNum % 26;
        var quotient = Math.floor(nNum / 26);
        if (modulo === 0) {
            result = letter(quotient - 1) + letter(26);
        } else {
            result = letter(quotient) + letter(modulo);
        }
    }

    return result;
}

function letter(nNum) {
    var a = "A".charCodeAt(0);
    return String.fromCharCode(a + nNum - 1);
}

It depends what do you mean by "streamlined"? There is not much you can do to, for example, reduce the complexity of your algorithm. What you can do is to increase the clarity of your algorithm, which falls into the domain of code review or improvement.

Maybe there is some math trick that can do the job faster, or with fewer lines of code. However there is also a good chance that such a trick will not make your algorithm look more "streamlined". It might even go the opposite direction and make it harder for the code to reveal its intention.

That being said, the problem you solved can be generalized to the problem of converting number to an arbitrary base, and then encode the number-in-new-base using a set of arbitrary symbols. In your case, the base is 26. The symbols are ["A", "B", ..., "Z"].
How a number is encoded or represented in a new base is independent of the base-convertion itself.

Looking at it this way, the only improvement I can think of is "do not limit the convertion to only one or two characters". Such a constraint does not bring much simplicity. But this probably is still implementation level improvement.

Your algorithm (or api) could eventually look like: number.convertToBase(26).encodeIn (["A", "B", ... , "Z"]). Is this more "streamlined"?


Exponentiation using while loop in Python I could not think of any better way to raise to power, except this, but how do I refactor it? I should only use while loop, not for loop, not x**y.

I shall not use pow(x,y)

result = 1
base = 3
counter = 1
degree = 4
while counter <= degree:
        result = base * result
        counter += 1
        print("counter %d and the result %d" %(counter, result))
print("result is ", result)

Your algorithm is \$O(n)\$ where \$n = \text{degree}\$.
This means the performance of your code is quite poor.
You can make an \$O(\log(n))\$ algorithm by using binary operators on the exponent.

For an explanation of this algorithm, you:

Have the exponent in a form where you can manipulate it's binary.
Whilst the exponent isn't empty:

If the last (smallest) index of the exponent is 1, times result by the base.
Remove the last index of the exponent.
Multiply the base by it's self.

Which results in:

def pow(base, exp):
    result = 1
    while exp:
        if exp & 1:
            result *= base
        exp >>= 1
        base *= base
    return result


Verifying if a car is a sports car based on speed and horse power I have an assignment where I am required to verify if a car is a sports car or not by minimum max speed and minimum horse power. I have finished the program and it works great. I was just wanting to see if you can review my assignment and see if there is room for improvement.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Project3
{
class sportsCar
{
    public int _maxSpeed = 0;
    public int _horsepower = 0;
    private static int _MinimumRequiredSpeed = 150;
    private static int _MinimumRequiredHorsepower = 250;

    public bool IsSportsCar()
    {
        if ((_maxSpeed >= _MinimumRequiredSpeed) && (_horsepower >= _MinimumRequiredHorsepower))
            return true;
        else
            return false;
    }
    public void maxSpeed(int newmaxSpeed)
    {
        _maxSpeed = newmaxSpeed;
    }
    public void horsepower(int newhorsepower)
    {
        _horsepower = newhorsepower;
    }
}

class Program
{
    static void Main(string[] args)
    {
        sportsCar car = new sportsCar();

        Console.Write("What is the max speed? ");
        int newmaxSpeed = int.Parse(Console.ReadLine());
        Console.Write("What is the max horsepower? ");
        int newhorsepower = int.Parse(Console.ReadLine());

        car.maxSpeed(newmaxSpeed);
        car.horsepower(newhorsepower);

        bool sportCar = car.IsSportsCar();
        if (sportCar == true)
        {
            Console.WriteLine("This is Definately a Sports Car");
        }
        else
        {
            Console.WriteLine("This is NOT a Sports Car");
        }
    }
}
}

_maxSpeed and _horsePower should be private.

You have (confusingly named) setter methods for public variables, but no getters.  That makes for an odd interface.

In C# you by convention use properties for getters and setters.  So instead of

public int _maxSpeed = 0;
public int _horsepower = 0;
public void maxSpeed(int newmaxSpeed)
{
    _maxSpeed = newmaxSpeed;
}
public void horsepower(int newhorsepower)
{
    _horsepower = newhorsepower;
}

You have

public int MaxSpeed { get; set; }
public int HorsePower { get; set; }

Since these values are unlikely to change, you could make the type immutable:

class Car
{
    public readonly int MaxSpeed;
    public readonly int HorsePower;

    public Car( int maxSpeed, int horsePower )
    {
        MaxSpeed = maxSpeed;
        HorsePower = horsePower;
    }
}

Instead of checking a boolean expression and then returning a literal true or false, just return the expression instead.  Booleans are a type just like any other.  Also, you don't need the comparison to true or false for the same reason.

if( someCondition == true )
    return true;
else
    return false;

You have

if( someCondition )
    return true;
else
    return false;

or better yet

return someCondition;

So

return _maxSpeed >= _MinimumRequiredSpeed &&
       _horsepower >= _MinimumRequiredHorsepower;

Whenever you take user input you need to check it.  Your program will crash if the user enters something that is not a number.  You can check this by using TryParse instead of Parse.

int horsePower;
do
{
    string input = Console.ReadLine();
}
while( !int.TryParse( input, out horsePower ) );

Other than that it is fine, keep at it ;)


duplicated operator(or attribute) on the same input control - v-model and @change I wrote a code like below - It is nuxt.js, which is one of the Vue frameworks. This code will be expected to

When change event triggered, update data value. There would be some logic to alter passed data
At the same time, it connected with data value to apply input box changing

<template>
  <div>
    <!-- I got question here -->
    <input type="text" ref="receiver" @change="applyData('receiver')" v-model="deliveryInfo.receiver" />
    <button type="button" @click="test">TEST</button>
  </div>
</template>
<script>
  export default {
    data: function () {
      return {
        deliveryInfo: {
          receiver: ''
        }
      }
    },
    methods: {
      applyData: function (refName) {
        let ref = this.$refs[refName];
        if (ref) {
          let temp = {};
          temp[refName] = ref.value;
          this.applyAddress(temp);
        } else {
          console.log("There is no " + refName + " in this.$refs");
        }
      },
      applyAddress: function (options) {
        this.deliveryInfo = Object.assign({}, this.deliveryInfo, options);
        this.$emit('applyChildData', this.deliveryInfo);
      },

      test: function () {
          console.log('set John Doe');
        this.deliveryInfo = Object.assign({}, this.deliveryInfo, {receiver: 'John Doe'});
        console.log('After change ', this.deliveryInfo);
        // It triggers applyData method and change input element value
        // But when I remove v-model in input value, it is not working
      }
    }
  }
</script>

And it works as expected.

However, using v-model and @change at the same tag looks weird to me since both of tag manipulates the connected value(in this case, it would be deliveryInfo.receiver)

Is this code OK to use in production project?
Thanks!

FYI, in my real case, it applied to a phone number. I want to show a phone number without dashes in a browser, but data should be saved with dashes.

So when people type in the input, @change alter data to insert dashes.

Responding to Your question

This is good use of ref and Object.assign() but normally this would be a good place to use a watched Property, or if you need to add dashes, a computed property.

Take a look at the example below, which uses a watcher, instead of the @onchange, which means that the method applyData() can be eliminated:

const form = new Vue({
    el: '#form',
    data: function () {
      return {
        deliveryInfo: {
          receiver: ''
        }
      }
    },
    watch: {
      'deliveryInfo.receiver': function(newValue, oldValue) {
        console.log('receiver changed - deliveryinfo:', this.deliveryInfo);
        this.$emit('applyChildData', this.deliveryInfo);
      }
    },
    methods: {
      test: function () {
          console.log('set John Doe');
        this.deliveryInfo = Object.assign({}, this.deliveryInfo, {receiver: 'John Doe'});
        console.log('After change ', this.deliveryInfo);
        // It triggers applyData method and change input element value
        // But when I remove v-model in input value, it is not working
      }
    }
  });

    <input type="text" ref="receiver" v-model="deliveryInfo.receiver" />
    <button type="button" @click="test">TEST</button>

Or the snippet below which uses a computed property to add a dash between the third and fourth characters...

const form = new Vue({
    el: '#form',
    data: function () {
      return {
        deliveryInfo: {
          receiver: ''
        }
      }
    },
    computed: {
      dashedReceiver: function() {
        if (this.deliveryInfo.receiver.length > 3) {
          return this.deliveryInfo.receiver.slice(0,3) + '-' + this.deliveryInfo.receiver.slice(3);
        }
        return this.deliveryInfo.receiver;
      }
    }
  });

    <input type="text" ref="receiver" v-model="deliveryInfo.receiver" />

{{dashedReceiver}}

Other feedback

One other critique I have is this line:

let ref = this.$refs[refName];

That variable never gets re-assigned so to avoid accidental re-assignment, use const instead of let. There are some who recommend the following:

  use const by default
  only use let if rebinding is needed
  1

There may not be much of a performance difference here although after reading some SO answers like this one and this one there is a potential but that might only be if a primitive type value (e.g. 42) was used.

1https://mathiasbynens.be/notes/es6-const


Verifying requirements before deleting a user-parent-student-school relationship I am new to node, after I finished my APIs I realized that all of them are a mess, and are a callback hell, this forced me to learn about promises, now it was all good until I faced an API that has condition with more than possible function, my question is how to do nested promises, my code is about a parent object who has a user attached to it, when delete route is called there are many conditions:

If this parent has children attached to it it shouldn't be deleted
If there are no children,

This parent is in more than one school, the school_id is removed from the school id of the parent object and the user object attached to it
If this parent has no children and only one school_id it should be removed and the user attached also should be removed

The code looks like this and it's 100% functional. I am trying to apply promises and make my code better.

router.post('/delete',Validation, function (req, res) {
var school_id = req.body.schoolId;
var parent_id = req.body.selected[0];

Student.findOne({parent_ids:parent_id},function(err,parentF){
    if(err){
        console.log(err);
        res.json({status:"error",message:"an error occurred"});
        return
    }else if(parentF){
        res.json({status:"error", message:"you can not delete a parent who has students associated with it"});
        return;
    }else{
        Parent.findOne({_id:parent_id},function(err,parent){
            if(err){
                console.log(err);
                res.json({status:"error",message:"an error occurred"});
                return;
            }else{
                if(parent.school_id.length>1){
                    var a = parent.school_id.indexOf(school_id);
                    parent.school_id.pop(a);
                    parent.save(function(err,parent){
                        if(err){
                            console.log(err);
                            res.json({status:"error",message:"an error occurred"});
                            return;
                        }else{
                            User.findOne({refid:parent_id},function(err,user){
                                if(err){
                                    console.log(err);
                                    res.json({status:"error",message:"an error occurred"});
                                    return;
                                }else {
                                    user.school_id.pop(a);
                                    user.save(function(err) {
                                        if(err){
                                            console.log(err);
                                            res.json({status:"error",message:"an error occurred"});
                                            return;
                                        }else{
                                            res.json({status: "success", message: "parent removed"});
                                            return;
                                        }
                                    });
                                }
                            });
                        }
                    });
                }else{
                    Parent.remove({_id: parent_id}, function (err) {
                        if (err) {
                            res.json({status: "error", message: err.message});
                        } else {
                            User.remove({refid:parent_id},function(err){
                                if (err) {
                                    res.json({status: "error", message: "parent user wasn't deleted"});
                                    return;
                                }else{
                                    res.json({status: "success", message: "parent data successfully deleted"});
                                    return;
                                }
                            });

                        }
                    });
                }
            }
        });
    }
});
   });

One of the biggest things you can do to clean up you code is to simply remove the unnecessary if-else blocks.  In many cases you have a return inside the first conditional, which means there is no reason for an else block at all.

For example, you code rewrite like this:

Student.findOne({parent_ids:parent_id},function(err,parentF){
    if(err){
        console.log(err);
        res.json({status:"error",message:"an error occurred"});
        return
    }
    if(parentF){
        res.json({status:"error", message:"you can not delete a parent who has students associated with it"});
        return;
    }
    Parent.findOne({_id:parent_id},function(err,parent){
        if(err){
            console.log(err);
            res.json({status:"error",message:"an error occurred"});
            return;
        }
        if(parent.school_id.length>1){
            var a = parent.school_id.indexOf(school_id);
            parent.school_id.pop(a);
            parent.save(function(err,parent){
                if(err){
                    console.log(err);
                    res.json({status:"error",message:"an error occurred"});
                    return;
                }
                User.findOne({refid:parent_id},function(err,user){
                    if(err){
                        console.log(err);
                        res.json({status:"error",message:"an error occurred"});
                        return;
                    }
                    user.school_id.pop(a);
                    user.save(function(err) {
                        if(err){
                            console.log(err);
                            res.json({status:"error",message:"an error occurred"});
                            return;
                        }
                        res.json({status: "success", message: "parent removed"});
                        return;
                   });
               });
           });
        } else {
            Parent.remove({_id: parent_id}, function (err) {
                if (err) {
                    res.json({status: "error", message: err.message});
                } else {
                    User.remove({refid:parent_id},function(err){
                        if (err) {
                            res.json({status: "error", message: "parent user wasn't deleted"});
                            return;
                        }
                        res.json({status: "success", message: "parent data successfully deleted"});
                        return;
                    });
                }
            });
        }
    });
});

Hmm... half my post went away :(

I had also mentioned that you should look into naming your functions as a way to get away from the nesting that can be proliferated by anonymous functions.  You could add named functions for handling you error and success logging/response formation.   You could also add named functions to perform the callbacks, so as to flatten these callbacks out as well as provide re-use.

Finally, I think you have some style problems:

inconsistent use of semicolons at end of line
inconsistent spacing around flow control structures
inconsistent spacing around commas in function arguments and object literals
inconsistent use of camelCase vs. snake_case (you should probably just stick to camel casing in JS)

You should consider defining a style and enforcing it.


Insert a number in a sorted list and return the list with the number at the correct index I feel like the title is a bit wordy. What I have defined here is a function that takes two parameters:

a list that contains valued sorted from smallest to biggest
a number to insert at the right index so that the returned list print its values from smallest to biggest

NOTE: Recursion is mandatory

def insert(lst, to_insert):
    """
    parameters : lst of type list, that contains values sorted from smallest to largest;
                 to_insert : represents a value
    returns : same list with the to_insert value positioned at the right index
              in order for the list to remain sorted from smallest to largest;
    """
    if len(lst) == 1:
        return []
    if lst[0] < to_insert and to_insert < lst[1]:
        lst[3] = to_insert
        return [lst[0]] + [lst[3]] + insert(lst[1:], to_insert)
    else:
        return [lst[0]] + insert(lst[1:], to_insert)

print(insert([1,2,3,4,5,7,8,9], 6))

The list outputs the following :

[1,2,3,4,5,6,7,8]   #not sure where 9 got left

How do I optimize this function, using only simple functions.

Recursion is usually a poor choice in Python.  Non-tail recursion is usually a poor choice in any language (and this is non-tail because we apply + to the result before returning it).

It's better to use the standard bisect module to find (once) the correct index at which to insert.  This module provides the bisect() function to locate the correct index, and also the insort() function that calls bisect and then inserts, exactly as we want:

insert = bisect.insort

This will insert in place, rather than creating a new list, so not exactly equivalent to the existing code.

As you say "using only simple functions", let's suppose you can't use the Standard Library (that's a bad assumption in general - Python philosophy is that it "includes batteries" to make your code simpler).  Then we can use a similar method to write our own non-recursive version.

def insert(lst, to_insert):
    """
    parameters : lst: sorted list (smallest to largest)
                 to_insert: value to add
    returns : copy of lst with the to_insert value added in sorted position
    """

    # binary search
    left = 0
    right = len(lst)

    while left != right:
        mid = left + (right-left) // 2
        if lst[mid] == to_insert:
            left = right = mid
        elif lst[mid] < to_insert:
            left = mid + 1
        else:
            right = mid

    # now copy the list, inserting new element
    return lst[:left] + [to_insert] + lst[left:]

Since the question states (without justification) that "recursion is mandatory", I recommend making the search recursive, but performing the insertion just once:

#! untested
def insert(lst, to_insert, left=0, right=None):
    if right is None:
        right = len(lst)

    if left == right:
        return lst[:left] + [to_insert] + lst[left:]

    else:
        mid = left + (right-left) // 2
        if lst[mid] == to_insert:
            left = right = mid
        elif lst[mid] < to_insert:
            left = mid + 1
        else:
            right = mid
        # move left or right, then
        return insert(lst, to_insert, left, right)

This is now tail-recursive, at least, and only reallocates the list elements once, rather than every level of recursion.


Big-O complexity calculation for a merge and sort function I have the following code which basically takes a list of sorted integer arrays and merges them together removing the duplicates.

I have tried to do the complexity analysis and came to a rough conclusion that it may be O(n2) order. Could you please see if my understanding is correct here.

Also, if you could suggest ways to make the code better in terms of its running time, I would really be grateful.

If you need the implementation of PrioQueue, please let me know.

public ArrayList MergeMultiple(Object[] input)
{
    ArrayList result = new ArrayList();
    int activeArrayIndex = -1;
    int inputArrayCount = input.Count();

    // Keep track of indices of the arrays
    int[] indices = new int[inputArrayCount];

    // Keep track of whether the end has been reached for each array
    bool[] endOfArray = new bool[inputArrayCount];

    // Heap
    PrioQueue heap = new PrioQueue();

    while (endOfArray.Contains(false)) // O(N)
    {
        for (int i = 0; i < inputArrayCount; i++) // O(N)
        {
            int[] active = (int[])input[i];
            int index = indices[i];
            if (activeArrayIndex == -1 || (i == activeArrayIndex && !endOfArray[i]))
            {
                int value = active[index];

                // Add value to the heap
                heap.Enqueue(i + "|" + value, value); // O(N)??
            }
        }

        // Add to result
        if (!heap.IsEmpty())
        {
            activeArrayIndex = activeArrayIndex == -1 ? 0 : activeArrayIndex;

            string activeArrayIndexWithVal = (string)heap.Dequeue(); // O(1)

            string[] activeArrayObject = activeArrayIndexWithVal.Split('|');

            // Identifying active array
            activeArrayIndex = Convert.ToInt32(activeArrayObject[0]);
            int[] activeArray = (int[])input[activeArrayIndex];

            // Adding value to result object if not already in the array list
            if (!result.Contains(activeArrayObject[1]))  // O(N)
            {
                result.Add(activeArrayObject[1]);
            }

            // Update index list with active array
            indices[activeArrayIndex]++;

            // update end of array count list for current array
            endOfArray[activeArrayIndex] = activeArray.Length == (indices[activeArrayIndex]);
        }
    }
    return result;
}

If N == number of input arrays and M == length of the longest input array then your outer while loop will run M times with the check condition being O(N) so your outer loop is O(MN). The inner loop runsNtimes and the heap insert isO(log(N))so your inner body isO(Nlog(N)). Resulting in a total complexity of O(M * N^2 * log(N))

Some review:

I find the entire code very confusing and hard to read. You keep track of different properties for the same array in disjoint helper arrays (indices and endOfArray) while they should be combined in a single state object.

Also your input is Object[] while obviously you expect a list of int[] so why not express that in your parameter directly like: List<int[]> input.

Your result is a non-generic ArrayList so it's really hard to know what the exact return type of the elements is. Reading through the code it looks like the elements get returned as strings which is probably very confusing for the user - why on earth would a method to which I pass a list of integer arrays return the merged result as an array of strings?

If I read your intention correctly then you want to merge the input arrays, remove all duplicates and return the result ordered. You can write that very nicely with LINQ:

public List<int> MergeLists(List<int[]> lists)
{
    return lists.SelectMany(l => l) // flatten into single collection
                .Distinct()         // remove duplicates
                .OrderBy(x => x)    // sort the result
                .ToList();          // build result list
}

Assuming all N arrays have the same maximum length M then the complexity should be:

Distinct() is in the order of O(N * M)
OrderBy() is in the order of O(N * M * log(N * M))
ToList() is in the order of O(N * M)

Which means the resulting complexity is O(N * M * log(N * M) so you have cut it down to linear in terms of number of input arrays and the code is significantly shorter, easier to read and maintain and very likely to contains fewer bugs.


Concatenating wide character strings I just wrote this code to concatenate two wide characters strings. Could someone please comment if it looks OK? (seems to work at least).

SomeStringClass amount = Window->get_text();
wchar_t strAmount[100] = L"Amount: "; // I know I could remove 100 from here too
wchar_t *ptr = wcscat( strAmount, amount.c_str() );
SomeStringClass  final;
final.set(ptr, 10 /* 10 is roughly due to length of Amount line 2*/ + amount.length());

Now, final contains what I wanted (e.g. "Amount: 123").

Firstly, avoid the non-n versions of (w)str_ functions. You've got a buffer overflow just begging to happen here - if amount.length() > 100 - 8, it'll overflow your buffer, causing undefined behaviour and general bad times. Use wcsncat instead:

static const unsigned kstrMax = 100;
wchar_t strAmount[kstrMax] = L"Amount: ";
wchar_t* ptr = wcsncat(strAmount, amount.c_str(), kstrMax - amount.length() - 1);

In fact, we have to be even more careful here, because there's still a bug in the above code (I left it in because I only caught it when modifying my example - it's easy to make mistakes). Say amount.length() > kstrMax, then you'd think the value kstrMax - amount.length() would be below zero, but being unsigned, it will silently overflow, and will instead be some very large positive value, hence we could still easily overwrite the buffer size with this if amount is large enough.

static const unsigned kstrMax = 100;
unsigned currentBufferSize = kstrMax;
wchar_t strAmount[kstrMax] = L"Amount: ";
if(amount.length() >= kstrMax - 8) {
    //find out the difference, remake your buffer and copy across
    currentBufferSize = newBufferSize;
}
wchar_t* ptr = wcsncat(strAmount, amount.c_str(), currentBufferSize - amount.length());

From the looks of it, the amount entered is meant to be a small numeric value. Never assume anything about input. Anything that a user touches must be sanitized before you blindly use it.

std::wstring would make this fiddling about go away:

std::wstring strAmount("Amount: ");
strAmount += amount.c_str();
final.set(strAmount.c_str(), strAmount.size());


Character Picture Grid (from Automate the Boring Stuff) Sorry, I'm new and not confident.  This is right, yes?  It seems to work but, again, other people's answers seem more complicated:
grid = [
    ['.', '.', '.', '.', '.', '.'],
    ['.', 'O', 'O', '.', '.', '.'],
    ['O', 'O', 'O', 'O', '.', '.'],
    ['O', 'O', 'O', 'O', 'O', '.'],
    ['.', 'O', 'O', 'O', 'O', 'O'],
    ['O', 'O', 'O', 'O', 'O', '.'],
    ['O', 'O', 'O', 'O', '.', '.'],
    ['.', 'O', 'O', '.', '.', '.'],
    ['.', '.', '.', '.', '.', '.']
]

row = 0
for entry in grid:
    for subentry in grid[row]:
        print(subentry, end = "")
    print("\n")
    row = row + 1

The question itself
It's not great. The textbook makes some decisions that haven't done the programmer any favours. First, you will notice that the input and output are mirrored about the line \$x=y\$. This is not a coincidence: it's the result of the textbook using column-major order when row-major order makes much more sense in this context.
Also, the grid presented is not only a sequence of sequences, it's a sequence of sequences of sequences since there are inner strings.
To solve both problems, ignore what the textbook tells you and instead attempt to code for this grid:
grid = (
    '..OO.OO..',
    # ...
)

and use [y][x] indexing instead of [x][y] indexing.
Such changes will reduce this to a much more trivial problem that does not require any explicit loops, and can be done via
print('\n'.join(grid))


Switching between queues in cellForIndexPath method My tableView is scrolling very slowly because there is a fetch request done in every cellForIndexPath method. I have verified this using instruments. The result of the fetch request determines the color of the label inside the cell.

Here is a solution I am using inside the custom cell:

func configureWith(myObject: MyObject) {
    titleLabel.text = myObject.text
    DispatchQueue.global(qos: .background).async {
        let color = self.getColor(forGroup: myObject.group)

        DispatchQueue.main.async {
            self.titleLabel.textColor = color
        }
    }
}

Is this a good solution?

Edit: Changing qos to userInitiated seems to work slightly better in that the scrolling is still virtually instant and the labels show the right color much faster.

This is not a good solution to handle fetch requests inside scrolling cellForIndexPath.

1 Fetch request should be asynchronous, because it may take time
2 Fetch request should be called in table view datasource and not in custom cell, because cell can be reused while scrolling
3 Should apply result of fetch request if cell is visible

Code in class where UITablView datasource is implemented-

func getColor(forGroup: Int, completion: @escaping (color: UIColor) -> Void) {
  //fetch request
  //request processing...
  completion(resultColor)
}

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
  let cell = tableView.dequeueReusableCell(withIdentifier: "identifier", for: indexPath) as! CustomCell
  let myObject = //object for indexpath
  cell.configureWith(myObject: myObject)
  self.getColor(forGroup: myObject.group) { (color) in
    DispatchQueue.main.async {
      if let cell = tableView.cellForRow(at: indexPath) {
        //this unwrapping is needed as tableview returns nil if cell is not visible
        cell.colorLable(color)
    }
  }
}

In CustomCell file:

func configureWith(myObject: MyObject) {
  titleLabel.text = myObject.text
}
func colorLable(_ color: UIColor) {
  self.titleLabel.textColor = color
}


Project Euler 22: Names scores by Loki 2 Alternative to Project Euler 22 solution.

Project Euler 22

Slightly more brittle than the original. This code depends on the input stream being exactly as specified. But on the other side of the coin it's slightly easier to read because you don't have to wonder about std::locale and code that uses facets like std::ctype<> (which few people understand how to use).

  Using names.txt (right click and 'Save Link/Target As...'), a 46K text
  file containing over five-thousand first names, begin by sorting it
  into alphabetical order. Then working out the alphabetical value for
  each name, multiply this value by its alphabetical position in the
  list to obtain a name score.

  For example, when the list is sorted into alphabetical order, COLIN,
  which is worth 3 + 15 + 12 + 9 + 14 = 53, is the 938th name in the
  list. So, COLIN would obtain a score of 938 × 53 = 49714.

  What is the total of all the name scores in the file?

#include <iostream>
#include <fstream>
#include <iterator>
#include <string>
#include <set>
#include <numeric>

long scoreName(std::string const& name)
{
    return std::accumulate(std::begin(name), std::end(name), 0L,
                           [](long v1, char x){return v1 + x - 'A' + 1;});
}

struct FirstName
{
    // Read a name from a stream.
    // Each name is quoted and separated with comma.
    // There are no extra spaces on the stream.
    //  Sample: "DEBBIE","APRIL","LESLIE","CLARA","LUCILLE","JAMIE"
    std::string name;

    // Read the next name from the input.
    // Note does not remove quotes.
    friend std::istream& operator>>(std::istream& s, FirstName& data)
    {
        return std::getline(s, data.name, ',');
    }

    // When we convert this object to a std::string
    // we remove the quotes around the name. In the code
    // below this happens as the iterator is de-reference into the set.
    operator std::string() const
    {
        return name.substr(1, name.size() - 2);
    }
};

Thus leaving a very simple main.

int main()
{
    std::ifstream   data("euler/e22.data");

    std::set<std::string> names{std::istream_iterator<FirstName>(data),
                                std::istream_iterator<FirstName>()};

    long    score   = 0;
    long    loop    = 1;
    for(auto name: names) {
        score += (loop * scoreName(name));
        ++loop;
    }
    std::cout << score << "\n";
}

std::accumulate with a lambda is quite a bit more complicated than writing out the range-for-loop manually, and it isn't even more concise:

long result = 0;
for(char c : name)
    result += c - 'A' + 1;
return result;

Using a std::set instead of a std::vector and sorting it once manually after reading everything in is possibly a bit less efficient. The really interesting part though is that duplicates are silently discarded, and the problem-statement doesn't seem to allow that.
Your range-for-loop in main needlessly copies every string. Better go for references even though that's slightly longer:

for(const auto& name: names)

Inserting a single char instead of a length-1 string is probably slightly more efficient.


Color vision checkup I have this old project I wrote years ago and it's all classes and methods all over the place. I am trying to learn/understand how I can apply a more OOP principle. It is very large and can't paste all the code here. Instead I will demonstrate what I have as a basic.

Checkup.cs

public class CheckUp
{
    //General information
    public string CliniciansName { get; set; }
    public string EmploymentStatus { get; set; }
    public DateTime DateOfCheck { get; set; }
    public string CheckUpType { get; set; }

    public ColourVision ColourVisionObject { get; set; }
    ///lots of other checkups like 45 and growing and forever changin
}

ColourVision.cs

public class ColourVision
{
    public int ColourVisionType { get; set; }
    public int OutcomeID { get; set; }
    public DateTime RecallDate { get; set; }
    public string Recommendations { get; set; }

    //Contains a few horrible methods to parse form other formats,
    // CSV datatables, XML, but needed. Here is one from a DataTable
    public Boolean ColourVision Load(System.Data.DataTable dt){

        Boolean isLoaded = false;
        System.Data.DataRow row = DataRow;
        try
        {
            ColourVisionType = row["ColourVision"].ToString();
            FFTOutcomeID = Convert.ToInt32(import.getOutcomeValue(row["ColourVisionOutcome"].ToString()));
            RecallDate = (row["ColourVisionRecallDate"] != DBNull.Value) ? (DateTime)row["ColourVisionRecallDate"] : DateTime.MinValue;
            Recommendations = (row["ColourVisionRecommendations"] != DBNull.Value) ? (string)row["ColourVisionRecommendations"] : "";
            isLoaded = true;
        }
        catch (Exception ex)
        {
            isLoaded = false;
        }
        return isLoaded;

     }

    //And this takes the data row and populates it self
    //by reference from healtchcheck, where there
    //are thousands of checks and lines of code
    //and this is what I am trying to streamline
    public void Update(ref Data.CheckUp checkUp)
}

The database is one table with over 400 columns and contains at least the General Info defined in CheckUp. It may or may not contain 0 more fields up to all the fields.

I am racking up my brain on how to create a Checkup Class, that can load the basic data form the data table, or 0 or more checkup objects, without manually calling methods like this for each kind of import logic.

if (ColorurVision.Load(ref dataRowFromSomethingElse) == true ) { CheckUp.ColourVision == null }

I am reading about Inheritance and the examples are clear but I just cannot for the life of me figure out how to do that here. I have a feeling this is perfect project for that. Maybe I am just looking at it wrong. Should I be rather inheriting from something else, like System.Data.DataTable and returning ColourVision, or CheckUp?

This is really a high level code review question based more on programming principles.

The biggest issues I see with this code is naming of Classes/Properties/Methods and proper separation of concerns.

I find the best way to think about separation of concerns is to think in terms of "has a" relationships to see if they make sense. For example, you might question whether a "Checkup has an employment status". When you put it that way, it's pretty clear that a Checkup doesn't make sense to have an employment status, but a Person might. It's pretty easy to fall into the trap of saying "I don't need to make a Person class because it's not that complex yet". This will come back to bite you later whether it's another developer that can't understand your code (maybe even yourself) or wishing you could more easily modify functionality because the elements are so tightly coupled.

Here's an example of how you might better organize your classes:

public class CheckUp
{
    public DateTime AppointmentTime { get; set; }
    public DateTime StartTime { get; set; }
    public DateTime EndTime { get; set; }

    public Clinician Clinician { get; set; }
    public Patient Patient { get; set; }

    // A type of test might be a ColorVisionTest.
    public List<Test> TestsRun { get; set; }
}

public class Person
{
    public String FirstName { get; set; }
    public String LastName { get; set; }
}

public class Clinician : Person
{
    public List<Licenses> Licenses { get; set; }
}

public class Patient : Person
{
    public List<Checkups> Checkups { get; set; }
}

public static class DatatableTestParser
{
    public static List<Test> ParseTests(DataTable dt)
    {
        // Parsing logic here.
    }
}

To keep harping on the separation of concerns, let's talk about a comment of yours:

  I am racking up my brain on how to create a Checkup Class, that can
  load the basic data form the data table

Typically you don't want to have something like a Checkup class also be concerned with parsing from things like a datatable. Again, a Checkup should only be concerned with being a Checkup and nothing more. You could have another class that did parsing from a DataTable or other data source, but the key is to separate it from the Checkup itself. Hopefully this helps.


Ribbon animation code too repetitive I spent the last two days getting a ribbon animation to work with divs so that I can later add li link elements to them to act as a menu. I used the progess bar animation code as inspiration from w3schools. The animation works great, only issue is that the code is very un-DRY. I tried to simplify the code by not repeating myself in the CSS and especially not in the javascript but I kept getting closure issues even after using self invoked functions. I would be greatful if anyone can give me some advice or even a solution that will greatly shorten my code to have the same animation effect. I know that I will have several ribbons on the page, each with different number of ribbon layers, so the code as it is can get very complicated very fast. N.B the code below is the working version.

function move1() {
  var width = 0;
  var elem = document.getElementById("ribbon-part-1");
  var id = setInterval(frame, 1);

  function frame() {
    if (width >= 100) {
      clearInterval(id);
    } else {
      width++;
      elem.style.width = width + '%';
    }
  }
}

function move2() {
  var width = 0;
  var elem = document.getElementById("ribbon-part-2");
  var id = setInterval(frame, 1)

  function frame() {
    if (width >= 100) {
      clearInterval(id);
    } else {
      width += 2;
      elem.style.width = width + '%';
    }
  }
}

function move3() {
  var width = 0;
  var elem = document.getElementById("ribbon-part-3");
  var id = setInterval(frame, 1)

  function frame() {
    if (width >= 100) {
      clearInterval(id);
    } else {
      width += 2;
      elem.style.width = width + '%';
    }
  }
}

function move4() {
  var width = 0;
  var elem = document.getElementById("ribbon-part-4");
  var id = setInterval(frame, 1)

  function frame() {
    if (width >= 100) {
      clearInterval(id);
    } else {
      width += 2;
      elem.style.width = width + '%';
    }
  }
}

function move5() {
  var width = 0;
  var elem = document.getElementById("end-ribbon");
  var id = setInterval(frame, 1)

  function frame() {
    if (width >= 100) {
      clearInterval(id);
    } else {
      width += 2;
      elem.style.width = width + '%';
    }
  }
}

function move() {
  setTimeout(move1, 300);
  setTimeout(move2, 600);
  setTimeout(move3, 900);
  setTimeout(move4, 1200);
  setTimeout(move5, 1500);
}
* {
  box-sizing: border-box;
}

.front-ribbon {
  position: relative;
  /*background-color: grey;*/
  height: 23px;
  width: 160px;
  z-index: 1;
  transform: skewY(-11deg);
}

.back-ribbon {
  position: relative;
  /*background-color: grey;*/
  height: 23px;
  width: 160px;
  transform: skewY(4.7deg);
  z-index: -1;
}

#ribbon-part-1 {
  background-color: rgb(70, 125, 76);
  height: 23px;
  width: 0%;
}

#ribbon-part-2 {
  background-color: rgb(89, 166, 101);
  height: 23px;
  width: 0%;
}

/*Including the code below will allow for a reverse progressive bar*/
.front-ribbon #ribbon-part-2 {
  display: block;
  float: right;
}

#ribbon-part-3 {
  background-color: rgb(70, 125, 76);
  height: 23px;
  width: 0%;
}

#ribbon-part-4 {
  background-color: rgb(89, 166, 101);
  height: 23px;
  width: 0%;
}

/*Including the code below will allow for a reverse progressive bar*/
.front-ribbon #ribbon-part-4 {
  display: block;
  float: right;
}

#end-ribbon {
  fill: rgb(70, 125, 76);
  width: 0%;
}

JavaScript ribbon animation

<svg id="end-ribbon">
  <path d="M-6 17 L35 24 L24 10 Z" />
</svg>

<button onclick="move()">Click Me</button>

A DRYer version of your code could be:

function prepare(elem, rate, width) {
    var width = 0;
    return function frame() {
        if (width >= 100) return;
        width += rate;
        elem.style.width = width + '%';
        setTimeout(frame, 5);
    }
}

function move() {
    ['ribbon-part-1', 'ribbon-part-2', 'ribbon-part-3', 'ribbon-part-4', 'end-ribbon'].forEach(function(id, i) {
            var el = document.getElementById(id);
            setTimeout(prepare(el, i == 0 ? 1 : 2), i * 300);
    });
}

jsFiddle: https://jsfiddle.net/wLwo0vs6/

I use the ids of the elements to determine how many parts/things that has to move and like that I don't have to hard code stuff. You could also get them from the DOM knowing the starting button that was pressed. This way you could use the exact same code for all the places you have.

But I don't like the animation not being synced properly, the correct for me would be that when one section reached 100% the next section would start the animation. So I suggest a modification that could be done like this:

function prepare(elems, width) {
    var width = 0;
    var elem = elems.shift();
    return function frame() {
        if (width >= 100) {
            elem = elems.shift();
            if (!elem) return;
            width = 0;
        } else {
            width++;
            elem.style.width = width + '%';
        };
        setTimeout(frame, 5);
    }
}

function move() {
    var elements = ['ribbon-part-1', 'ribbon-part-2', 'ribbon-part-3', 'ribbon-part-4', 'end-ribbon'].map(function(id) {
        return document.getElementById(id);
    });
    setTimeout(prepare(elements), 300);
}

jsFiddle: https://jsfiddle.net/db8hbpry/

The missing part is the logic for accelerating. If you know what you want there I can adjust.

More than this, small things could be more pressed, but I think it would reduce readability.


Implementation of the Jacobi Symbol in C I am new to C programming and wrote the following algorithm for the calculation of the Jacobi Symbol. Though there are some faster versions of this algorithm available I am only looking for some feedback on my coding style. I should mention that I was taught to ONLY USE ONE return statement in functions and NOT to use things like break, continue or go-to. This is a "philosophy" I also believe in.

How could I improve?
Is anything unclearly formulated?

One point I could think of would be to use "long long" instead of only "long" since this algorithm is used in cryptography and should thus be suited for very large integers.

Any kind of feedback would be welcome.

EDIT: There is still a (commented) print statement in this code that helps to visualize the algorithm.

#include <stdio.h>
#include <stdlib.h>

/*
+*****************************************************************************

* Function Name:    jacobi

* Function:         Calculate the Jacobi Symbol

* Arguments:        a   =   numerator
                    n   =   denominator

* Return:           (a|n)

* Notes:            This function evaluates the Jacobi Symbol for two integers
                    a and n. It is based on the same idea as the Euclidean
                    alogrithm for the gcd. The bit complexity is O(log^2(p)).

-****************************************************************************/

short jacobi(long a, long n)
{
    // The two if-conditions below terminate the function in case of wrong inputs, i.e.: n < 3  or 2 | n.
    if (n<3) {
        printf("Wrong input for the Jacobi Symbol: The denominator has to be greater than 2.\n");
        exit(0);
    }
    if (n%2 == 0) {
        printf("Wrong input for the Jacobi Symbol: The denominator has to be coprime to 2.\n");
        exit(0);
    }
    long n_mod8 = 0;
    short result = 1;
    short condition = 1;
    while (condition == 1) {
        // printf("(%d|%d) and result = %d \n", a, n, result);
        a = a % n;
        // The The if-condition below utilises the fact that (0|n) = 0.
        if (a == 0) {
            condition = 0;
            result = 0;
        } else {
            // The while-loop below utilises the Second Supplement
            // to extract the 2’s out of a.
            while (a % 2 == 0) {
                a = a / 2;
                n_mod8 = n % 8;
                if (n_mod8 == 3 || n_mod8 == 5) {
                    result = -result;
                }
            }
            // The if-condition below utilises the fact (1|n) = 1.
            if ( a == 1) {
                condition = 0;
            } else {
                // This else-condition utilises the Law of
                // Quadratic Reciprocity to transfer (a|n) to (n|a).
                long tmp = a;
                a = n;
                n = tmp;
                if (a % 4 == 3  &&  n % 4 == 3) {
                    result = -result;
                }
            }
        }
    }
    return result;
}

int main()
{
    printf("%d", jacobi(4852777,12408107));
    return 0;
}

```

I am only looking for some feedback on my coding style.

Formatting is good. I hope it is auto formatted.

Respect the presentation width

Rather than oblige a horizontal scroll bar, auto format to a narrower width to avoid that.

Avoid dogma

"to ONLY USE ONE return statement in functions and NOT to use things like break, continue or go-to." --> This is a reasonable goal, but better to code for clarity.  Use break, continue and even sometimes goto when it provides cleaner code.

  How could I improve?

Use short to save space in arrays - not here

short jacobi(long a, long n) returns 0,1,-1.  There is no space, code, performance savings expected with short.  Recommend returning int here. int is often the optimal type for an integer.

// short result = 1;
int result = 1;
...
return result;

Cryptography begs for unsigned types

Instead of long, consider the widest unsigned type of uintmax_t or the widest fixed size type: uint64_t.

Unsigned types will improve code performance.

long n ... n_mod8 = n % 8; requires a division as n % 8 has 15 different results [-7 ... 7].  With unsigned long n, n % 8 is simply a mask operation.

Consider bool

For clarity, use bool.

//short condition = 1;
//while (condition == 1) {
bool condition = true;
while (condition) {

Remove debug code

e.g. // printf("(%d|%d) and result = %d \n", a, n, result);.

Error messages: consider stderr

// printf("Wrong input for the Jacobi Symbol:...
fprintf(stderr, "Wrong input for the Jacobi Symbol: ...

Post expected test code results

What should printf("%d", jacobi(4852777,12408107)); print?

Consider a few more test cases.

Post compilable code

Trailing ``` breaks compilation.

Minor

// The The if-condition below utilises the fact that (0|n) = 0.
to
// The if-condition below utilises the fact that (0|n) = 0.

  Is anything unclearly formulated?

I'd add a reference for Jacobi symbol.

// https://en.wikipedia.org/wiki/Jacobi_symbol

n == 1?

Hmmm. Jacobi symbol allows n==1.  Code here does not.


Unique elements of a list and corresponding indices in Haskell Given a list vs, I want to get the list vs' of the unique elements of vs, as well as the indices of the elements of vs in vs'. For example, if vs = [7, 8, 7, 8, 9] I want to get [7,8,9] (the unique elements) and [0,1,0,1,2] (the indices).

An easy implementation is:

import           Data.List
import           Data.Maybe

unique :: Eq a => [a] -> ([a], [Int])
unique vs = (vsnub, indices)
  where
  vsnub = nub vs
  indices = map (\v -> fromJust $ elemIndex v vsnub) vs

However I think this is not efficient. I've done an implementation using mutable vectors. The pseudo-code is (saying that vs is a list of numbers):

n = length of vs
idx = list of n integers (to store the indices)
visited = [false, false, ..., false] (n elements)
nvs = list of n numbers (to store the unique elements)
count = 0
for(i = 0; i < n; ++i)
{
  if(not visited[i])
  {
    nvs[count] = vs[i]
    idx[i] = count
    visited[i] = true
    for(j = i+1; j < n; ++j)
    {
      if(vs[j] = vs[i])
      {
        visited[j] = true
        idx[j] = count
      }
    }
    count ++
  }
}
nvs = first 'count' elements of nvs

And here is my Haskell code:

{-# LANGUAGE ScopedTypeVariables #-}
import           Control.Monad               ((>>=))
import           Data.Vector.Unboxed         (Unbox, Vector, freeze, (!))
import           Data.Vector.Unboxed.Mutable (IOVector, new, write)
import qualified Data.Vector.Unboxed.Mutable as VM

unique' :: forall a . (Unbox a, Eq a) => [a] -> IO (Vector a, Vector Int)
unique' vs = do
  let n = length vs
  idx <- VM.replicate n 0 :: IO (IOVector Int)
  visited <- VM.replicate n False :: IO (IOVector Bool)
  nvs <- new n :: IO (IOVector a)
  let inner :: Int -> Int -> Int -> IO ()
      inner i j count | j == n = return ()
                      | otherwise =
                        if vs !! i == vs !! j
                          then do
                            write visited j True
                            write idx j count
                            inner i (j+1) count
                          else inner i (j+1) count
  let go :: Int -> Int -> IO (IOVector a)
      go i count | i == n = return $ VM.take count nvs
                 | otherwise = do
                   vst <- VM.read visited i
                   if not vst
                     then do
                       write nvs count (vs !! i)
                       write idx i count
                       write visited i True
                       _ <- inner i (i+1) count
                       go (i+1) (count + 1)
                     else go (i+1) count
  nvs' <- go 0 0 >>= freeze
  idx' <- freeze idx
  return (nvs', idx')

Is it nice? Can it be improved? Is there a solution that does not resort to IO?

Various Comments

First, Stack Exchange sent the body of my response into the void when I logged in to post it, so I'm going to be a bit more brief and a bit less organized than I originally wanted because I'm lazy.

This is not really reviewing your code, so it would've been better served as a comment, but I didn't think to comment my approach before I wrote out all the code. Might as well share it in case it's helpful.

To be honest, I don't quite understand your second solution, but on a second read it looks like it might still be n^2 (same asymptotic complexity as your first solution), so that would explain why it's slower. I assumed at first it was just something to do with mutability and vectors (which I don't really understand myself), but on second read I'm no longer so sure about this.

If you don't mind me being frank in answering your first question: I don't really think it's nice. I had a pretty rough time reading it, to the point where I instead just wrote a more Haskell-inclined solution that has better asymptotic performance than your first one.

Proposed Solution

Are you okay with an n log n solution? If memory serves, with the right choice of set/map, the log's base is so large that it is effectively linear.

Here is one such solution

import Data.List (foldl', sortOn)
import Data.Map (Map)
import qualified Data.Map as Map

-- | Gives the unique elements of 'elems' in order.
orderedNub :: Ord a => [a] -> [a]
orderedNub elems = sortOn (firstIndexMap Map.!) (Map.keys firstIndexMap)
  where
    -- Insert such that if the value is already in 'firstIndexMap', it is not
    -- updated.
    addElem m (elem, index) = Map.insertWith (flip const) elem index m
    -- Left fold in order to get the first occurance of each element.
    firstIndexMap = foldl' addElem Map.empty $ zip elems [0..]

-- | Gives the unique elements of 'elems' and the indices of 'elems' in
-- the unique list of elements.
unique :: Ord a => [a] -> ([a], [Int])
unique elems = (uniques, indices)
  where
    uniques = orderedNub elems
    uniqueInds = zip uniques [0..]
    indexMap = foldr (uncurry Map.insert) Map.empty uniqueInds
    -- We can use unsafe indexing since we know that 'indexMap' has
    -- the right values.
    indices = [indexMap Map.! x | x <- elems]

Some Notes About the Solution

I changed the code so it is correct now, assuming you want to preserve order. Some of these comments may be a bit out of date as a result.
orderedNub' is a kind of clunky definition, but it should have better asymptotic performance than nub. I’m pretty sure you can avoid the overhead of an actual sort (one clunky way would be to use a vector like you’re using).
uncurry Map.insert is a cute pointfree thing, if that's not your dig just sub it with a lambda or helper function.
I can't think of a good way to avoid using Map.!, even though it's partial. If I used Map.lookup, then there'd have to be a fromJust or a Maybe overhead, even though the map is, by construction, going to have the right keys. Guess this is a shortcoming of the type system.


Maximum sum combination I am trying to solve this question: https://www.codechef.com/problems/TADELIVE

  Andy and Bob are the only two delivery men of Pizza-chef store. Today,
  the store received N orders. It's known that the amount of tips may be
  different when handled by different delivery man. More specifically,
  if Andy takes the ith order, he would be tipped Ai dollars and if Bob
  takes this order, the tip would be Bi dollars.

  They decided that they would distribute the orders among themselves to
  maximize the total tip money. One order will be handled by only one
  person. Also, due to time constraints Andy cannot take more than X
  orders and Bob cannot take more than Y orders. It is guaranteed that X
  + Y is greater than or equal to N, which means that all the orders can be handled by either Andy or Bob.

  Please find out the maximum possible amount of total tip money after
  processing all the orders

  1 ≤ N ≤ 10⁵
  1 ≤ X, Y ≤ N
  X + Y ≥ N
  1 ≤ Ai, Bi ≤ 10⁴

I have used a dynamic programming approach:

"""
https://www.codechef.com/problems/TADELIV]
"""

"""
https://www.codechef.com/problems/TADELIV]
"""

from functools import lru_cache

@lru_cache(maxsize=None)
def optimal_sum(x, y, i):
    if i >= len(a):
        return 0
    if x == 0:
        return sum(b[i:])
    if y == 0:
        return sum(a[i:])
    sum_with_a = a[i] + optimal_sum(x-1, y, i+1)
    sum_with_b = b[i] + optimal_sum(x, y-1, i+1)
    return max(sum_with_a, sum_with_b)

n, x, y = map(int, input().split())
a = [int(i) for i in input().split()]
b = [int(i) for i in input().split()]
ans = optimal_sum(x, y, 0)
print(ans)

Sadly this timed out :( How do I make it run faster?

Often it is the case that reformatting the data leads to insights. Let's take a look at the test case:

A: 1 2 3 4 5
B: 5 4 3 2 1

Now let's suppose that we subtract the smaller of each pair from both, and take the sum of the smallers:

A: 0 0 0 2 4
B: 4 2 0 0 0

If we can find the maximum tips available in this problem, the real result will be that amount plus 1 + 2 + 3 + 2 + 1.

Can you now see how to solve the problem more easily?

Unfortunately it appears that the tags are misleading; dynamic programming does not buy you much, but a greedy algorithm does.

  Your greedy algorithm works out for that test-case. I'm not sure if it works in general. Could you expand a bit on the intuition?

First off, it should be clear that we can always reduce the problem to one where the columns are of this form where one or both are zero; "subtract the smaller from both" always works.

So the problem now is: we must choose up to X numbers from row A, and up to Y numbers from row B, such that their sum is maximized.

The proposed algorithm is: always take the largest number remaining. Suppose we have:

A: 0 0 2 1 5  -- choose up to 2
B: 4 2 0 0 0  -- choose up to 4

We now have to make assignments, and we do it in order from highest to lowest benefit. Start with the biggest bang:

           A
A: 0 0 2 1 5
B: 4 2 0 0 0

Then the next:

   B       A
A: 0 0 2 1 5
B: 4 2 0 0 0

Now we have a tie, so pick both:

   B B A   A
A: 0 0 2 1 5
B: 4 2 0 0 0

And now we've exhausted A, which leaves

   B B A B A
A: 0 0 2 1 5
B: 4 2 0 0 0

What's the intuition that the greedy algorithm works?  Examine the first choice. If, say, we gave Bob the 5/0 delivery and take the zero tip, what does that get us? It only gives Andy the opportunity to make the lower-valued "1/0" delivery later on, which is not a win; we've given up five to gain one.


Reverse String in Java I've been revamping on my coding puzzles practices. What is the complexity (both time and space) for this method?

public static String reverse(String word) {

    if (word == null || "".equals(word) || word.length() == 1) {
        throw new IllegalArgumentException("Invalid Entry");
    }

    StringBuilder result = new StringBuilder();

    for (int i=word.length() - 1; i >= 0; i--) {
        result.append(word.charAt(i));
    }

    return result.toString();
 }

Is this a more optimized solution (what is the time and space complexity of this one as well):

public static String reverse ( String s ) {
    int length = s.length(), last = length - 1;
    char[] chars = s.toCharArray();
    for ( int i = 0; i < length/2; i++ ) {
        char c = chars[i];
        chars[i] = chars[last - i];
        chars[last - i] = c;
    }
    return new String(chars);
}

I know that there might be duplicate questions similar to this but am not asking for a solution. I'm only seeking how to figure out the two different types of complexities.

Linear in space

Both are linear in space.

The first one is linear because the StringBuilder makes a copy of the string.

The second one is linear because toCharArray makes a copy of the string.  We know it can't use the backing array of the string, because the string is immutable.  Clearly you can modify the character array.  We can ignore the swap variable (c), as it is either constant space or

We can consider a Java compiler (even if none currently work this way) that would release the backing array to the toCharArray, but we can't guarantee that.  Because the calling code may want to use the string after calling this method.  So the assumption in the method is that we are creating a new array.

If the input is a string and the output is a different string (and we can't change the original string, so it has to be different), then linear time is the best we could possibly do.  So even without the intermediate variable, these would still be linear in space.  Both create new strings.

Linear in time

Both are linear in time.

The first one does \$n\$ iterations with one append operation per iteration.  The append operations should be constant time.  There may be occasional copy operations that can be amortized to be constant time per append operation or linear time overall.  That's \$\mathcal{O}(n)\$.

The second one does \$\frac{n}{2}\$ iterations with two array assignments per iteration.  That's also linear, \$\mathcal{O}(n)\$.  Because \$\frac{n}{2}\$ grows linearly with \$n\$ and two is a constant.

Constant space

To have a method be constant in space, it needs to return the same memory that brings the input.  E.g.

public void reverse(char[] chars) {
    for (int i = 0, j = chars.length - 1; i < j; i++, j--) {
        char temp = chars[i];
        chars[i] = chars[j];
        chars[j] = temp;
    }
}

This is constant in space and linear in time.  But it neither takes nor returns a string.

Constant space and time

There's a sort of backwards way of reversing a string in constant time and space.

class ReversedString {

    private String string;

    public ReversedString(String string) {
        this.string = string;
    }

    public char charAt(int index) {
        return string.charAt(string.length() - 1 - index);
    }

}

But we wouldn't be able to just use this as a string without creating a new string.  The only operation that works (so far) is charAt.  We might be able to make other operations work, but not all of them.  In particular, a toString would be linear in time and space, just like the original methods.  Because it would have to make a new string of the same length.


Find largest block in matrix (Largest block)
Given a square matrix with the elements 0 or 1, write a program to find a maximum square submatrix whose elements are all 1s. Your program should prompt the user to enter the number of rows in the matrix. The program then displays the location of the first element in the maximum square submatrix and the number of the rows in the submatrix.

Here is a sample run:

Enter the number of rows in the matrix:
5

Enter the matrix row by row:
1 0 1 0 1
1 1 1 0 1
1 0 1 1 1
1 0 1 1 1
1 0 1 1 1

The maximum square submatrix is at (2, 2) with size 3

My solution is this:
(I didn't make it prompt an input because my computer hasn't arrived and the ide on my phone can't use text files as input. I didn't want to type in all the numbers to try different inputs.)
public class Main {
    public static void main(String[] args) {
        int[][] matrix = {
            {1, 0, 1, 0, 1},
            {1, 1, 1, 0, 1},
            {1, 0, 1, 1, 1},
            {1, 0, 1, 1, 1},
            {1, 0, 1, 1, 1}
        };

        //Check each location for sub-squares
        int[] largestBlock = new int[3];
        int maxSize = 0;
        for (int i = 0; i < matrix.length - 1; i++) {
            for (int j = 0; j < matrix[i].length - 1; j++) {
                int squareSize = 0;
                while (isSquare(i, j, squareSize, matrix)) {
                    squareSize++;
                }

                if (squareSize > maxSize) {
                    largestBlock[0] = i;
                    largestBlock[1] = j;
                    maxSize = squareSize;
                }
            }
        }

        largestBlock[2] = maxSize + 1;
        System.out.println("The first largest sub-square is located at (" + largestBlock[0]
          + ", " + largestBlock[1] + ") and is of size " + largestBlock[2]);
    }

    public static boolean isSquare(int row, int column, int squareSize, int[][] matrix) {
        int size = squareSize + 2;

        if (row + size - 1 >= matrix.length || column + size - 1 >= matrix.length)
            return  false;

        for (int i = row; i < row + size; i++) {
            for (int j = column; j < column + size; j++) {
                if (matrix[i][j] != 1)
                    return false;
            }
        }
        return true;
    }
}

There is a neat trick in many computer algorithms called memoization - the remembering of critical aspects of one computation, to reuse them in another. Memoization can help a lot in this problem. To explain this, consider the basic algorithm you have...

start at the top-left of the matrix
scan every position in the matrix
for each position, check whether it is the top-left of an increasing square of set values.
keep track of the maximum.

This process is effective, but not very efficient. What if I were to tell you there is a way to solve the problem without having to do step 3, just by remembering what you did in previous iterations of step 1 and 2?

The way we do things, is to remember the size of previously-computed squares of set positions. We remember the previous row, and the current row only. No need to remember more than that....

Looking at your example:

1 0 1 0 1
1 1 1 0 1
1 0 1 1 1
1 0 1 1 1
1 0 1 1 1

It is a matrix of size 5. We create two temporary arrays to record the previous and current memoization:

int[] previous = new int[5];
int[] current = new int[5];

Now, we compute the values for the current memoization - it works like this:

if the cell in the current column is not set, then the square at the current position is size 0.
if the cell is set, then it is 1 more than the smallest square above , above-left, or left of it. This needs a picture.....

As you can see, I have marked 3 squares - red, blue, and purple. Each of them is 2x2. The position at the bottom right, is the end of the 3x3 square. it is 1 + the minimum size of the blue, red, and purple squares.

So, it becomes relatively simple to identify squares, we only need to check three values for each cell.... the code is surprisingly simple, even if the concept is hard...

private static Square getLargestSquare(int[][] matrix) {
    if (matrix == null || matrix.length == 0) {
        return null;
    }

    final int height = matrix.length;
    final int width = matrix[0].length;

    Square max = null;

    // cheat, here, and use the first matrix row as 'current'
    // this will become 'previous' in the loop, and will not be changed.
    // note that the y-loop starts from 1, not 0.
    int[] previous = null;
    int[] current = matrix[0];

    for (int y = 1; y < height; y++) {
        // prepare the memoization space.
        // Forget the previous, move current back, and make a new current
        previous = current;
        current = new int[width];
        for (int x = 0; x < width; x++) {
            if (matrix[y][x] == 1) {
                int span = 1;
                if (x > 0) {
                    // no need to check the left-most column, if set, it is always size 1.
                    span = 1 + Math.min(current[x - 1], Math.min(previous[x], previous[x - 1]));
                }
                if (max == null || span > max.size) {
                    // because we find the max at x, and y, which are the bottom-right,
                    // we need to subtract the span to get to the top-left instead.
                    max = new Square(x - span + 1, y - span + 1, span);
                }
                current[x] = span;
            }
        }
    }

    return max;
}

Note that I have created a container-class Square to hold the result:

private static final class Square {
    private final int x, y, size;

    public Square(int x, int y, int size) {
        super();
        this.x = x;
        this.y = y;
        this.size = size;
    }

    @Override
    public String toString() {
        return String.format("Square at (%d,%d) is size %d", x, y, size);
    }

}

Additionally, the main method is now simple:

public static void main(String[] args) {
    int[][] matrix = {
            {1, 0, 1, 0, 1},
            {1, 1, 1, 0, 1},
            {1, 0, 1, 1, 1},
            {1, 0, 1, 1, 1},
            {1, 0, 1, 1, 1}
        };

    Square sq = getLargestSquare(matrix);

    System.out.println("Largest square: " + sq);
}

You can see the whole thing on ideone: Largest Square


Make roads contrast with Google Maps EDIT: I turned down the brightness on my monitor using the graphics options for the video card. This solved the issue. How this setting changed is a mystery...
I am leaving this up because googling it leads down some difficult and unhelpful paths.
ORIGINAL:
Google maps has become washed out. I cannot see roads. I have made a hack work around that dims the page to make the roads contrast with the map
See https://www.youtube.com/watch?v=MAPfjlwMyB4 for example of issue I am having.
Now, I made a snippet inside the developer console in chrome to "fix" this, but I am certain there are better ways to do it.
Here is the snippet:
var style = document.createElement('style');
style.type = 'text/css';
style.innerHTML = '.mapsTactileClientSettingsMain__widget-settings-offscreen .widget-settings-shim { width: 100%; height: 100%; opacity: 0.3; pointer-events: none; }';
document.getElementsByTagName('head')[0].appendChild(style);

This only works AFTER I have accessed the hamburger menu at least once. If I run the script before then, it has no effect.
The script simply uses the same overlay that google uses to dim the map while the menu is open. But it changes it to be "on" even when you aren't accessing the menu. It also disables mouse interaction with the overlay so the user can interact with the map.
I am looking for improvements or entirely different fixes that are better.
Thank you.

I am looking for ... entirely different fixes that are better.

Turn the brightness down using your operating system, graphics card software, or monitor.


Class to create HTML heading tags in React I have a class which is part of my page layout. It is creating header tag elements (h1 - h5). Could someone tell me how I could improve it (I know it should be improved, but don't know how)? I have problem with rewriting almost the same code in switch cases. Would rather to create some kind of <h{type}></h{type}> code, but couldn't.

import React, {Component} from 'react';
import PropTypes from 'prop-types';

export class TypographyHeader extends Component {
    createHeader() {
        let txt        = this.props.text;
        let spaceIndex = txt.indexOf(' ');
        let p2         = txt.substr(spaceIndex);

        txt =

                {txt.substr(0, spaceIndex)}
            ;

        switch(this.props.headerType) {
            case 1:
                return
{txt} {p2}
;
            case 2:
                return
{txt} {p2}
;
            case 3:
                return
{txt} {p2}
;
            case 4:
                return
{txt} {p2}
;
            case 5:
                return
{txt} {p2}
;
        }

        return
{txt} {p2}
;
    }

    render() {
        return (

                {this.createHeader()}

        );
    }
}

TypographyHeader.propTypes = {
    headerType: PropTypes.oneOf([1, 2, 3, 4, 5]).isRequired,
    text: PropTypes.string.isRequired,
};

Choosing the heading level at runtime:

According to the official docs, you cannot use expressions within JSX types. However, you can use capitalized variables:

const Heading = "h" + 1;
return <Heading>...</Heading>;

Further suggestions:

Rename TypographyHeader to TypographyHeading to avoid confusion with .
Rename headerType to level.
Rename txt to text in let txt = this.props.text for consistency's sake.
Don't recycle the txt variable by rebinding it to a JSX .
Consider using const variable bindings.
As there are 6 official heading levels, you might want to relax your propTypes constraints:

headerType: PropTypes.oneOf([1, 2, 3, 4, 5, 6]).isRequired

Use destructuring arguments to avoid writing let txt = props.text.
Make the white space following the  element obvious by appending {" "}.
Handle heading texts without spaces by splitting according to this suggestion.

Proposed refactoring:

function TypographyHeading({text, level}) {

    // Split text into leading and remaining words:
    const space = text.indexOf(" ");
    const leading = text.substr(0, space);
    const remaining = text.substr(space + 1);

    // Select heading
 -
:
    const Heading = "h" + level;

    return (

            <Heading className="typography_header">

                    {leading}

                {" "}{remaining}
            </Heading>

    );
}


Copy Multiple Excel Tables to VBA Dictionary I have  a number of Excel tables that I want to populate into VBA dictionaries. Most of the code is repetitive but there are some unique things for each table that prevent me from making one general purpose routine and calling it for each table. The basic code:

Public Sub MakeDictionary( _
    ByVal Tbl As ListObject, _
    ByRef Dict As Scripting.Dictionary)

    ' This routine loads an Excel table into a VBA dictionary

    Dim ThisList As TableHandler
    Set ThisList = New TableHandler

    Dim Ary As Variant

    If ThisList.TryReadTable(Tbl, False, Ary) Then
        Set Dict = New Scripting.Dictionary

        pInitialized = True

        Dim I As Long
        For I = 2 To UBound(Ary, 1)
            Dim ThisClass As MyClass
            Set ThisClass = New MyClass

            ThisClass.Field1 = Ary(I, 1)
            ThisClass.Field2 = Ary(I, 2)

            If Dict.Exists(Ary(I, 1)) Then
                MsgBox "Entry already exists: " & Ary(I, 1)
            Else
                Dict.Add Ary(I, 1), MyClass
            End If

        Next I

    Else
        MsgBox "Error while reading Table"
    End If

End Sub

TableHandler and TryReadTable are a class and function that reads a table into a variant array.
Field1 and Field 2 are the columns of the Excel table. My tables range from 1 to 8 columns depending on the table.

I don't know how to make these lines generic:

            Dim ThisClass As MyClass
            Set ThisClass = New MyClass

            ThisClass.Field1 = Ary(I, 1)
            ThisClass.Field2 = Ary(I, 2)

At this point I have to make 10-12 versions of MakeDictionary where only the class name and class fields are unique. If that's the best way to do it, I can live with it, but, I'd like to make my code more general.
Any suggestions?

Option Explicit!

Suggestion #1: Put Option Explicit at the top of the module. Always!

This would have prevented the very embarrassing error of Dict.Add Ary(I, 1), MyClass. Upon which I see that this code would neither compile nor run unless this Class is a default instance.

Function or Sub?

If you are only returning one value, then you can use a Function, not a Sub.

Keep it Simple!

You have made this more complicated than required. You already pass in a ListObject, and you want the DataRange. Just do that instead of calling custom classes that return something that you then have to modify/clean up.

Dim ThisList As TableHandler
Set ThisList = New TableHandler

Dim Ary As Variant

If ThisList.TryReadTable(Tbl, False, Ary) Then
    [...]

becomes

Dim Ary As Variant
If Not Tbl is Nothing then
    Ary = Tbl.DataRange
    [...]

In addition, you have pInitialized = True but you don't declare or use pInitialized anywhere else.

Interface or encapsulate?

This is a classic example where the use of an interface would greatly assist.
Create an interface class which basically sets up a contract for some of the methods to be used. In other words, make sure that all of these 10-12 Classes you mention have the right method or property so that you can call and create as required.

Public Sub MakeDictionary( _
    ByVal Tbl As ListObject, _
    ByRef Dict As Scripting.Dictionary)

Can become

Public Sub MakeDictionary( _
    ByVal Tbl As ListObject, _
    ByRef Dict As Scripting.Dictionary,
    ByVal SomeTag as <whatever type is most appropriate>)

And further in the code you can have

    For I = LBound(Ary, 1) To UBound(Ary, 1) '<- remember - we fixed this in my comment above!
        Dim ThisClass As IClass
        Select Case SomeTage
            Case TagA
                Set ThisClass = New MyClass1
            Case TagB
                Set ThisClass = New MyClass2
        End Select

        'ThisClass.Field1 = Ary(I, 1)
        'ThisClass.Field2 = Ary(I, 2)
        ThisClass.RelevantFillMethod(Ary(I, 1),Ary(I, 2))

        If Dict.Exists(Ary(I, 1)) Then ' Just in case there is some sort of transform?
            MsgBox "Entry already exists: " & Ary(I, 1)
        Else
            Dict.Add Ary(I, 1), ThisClass
        End If
    Next I

Now, if the Interface had a factory method which created a new version of the relevant class - you could dispense with the Dim and pass the MyClass1 instance as a parameter, calling MyClass1.New as necessary.

' Class IClass
Option Explicit

Public Function NewMe() As IClass
End Function

Public Sub RelevantFillMethod(val1 As Variant, val2 As Variant)
End Sub

And

' Class Class1
Option Explicit
Implements IClass

Private p_V1 As Variant
Private P_V2 As String

Public Function EverythingElse()
    '[...]
End Function

Private Function IClass_NewMe() As IClass
    Set IClass_NewMe = New Class1
End Function

Private Sub IClass_RelevantFillMethod(val1 As Variant, val2 As Variant)
    p_V1 = val1
    P_V2 = CStr(val2)
End Sub

Subs and functions!

Of course, you can make this a bit easier by addressing the single responsibility concept and using logical functions. However, in the OP there is not enough information to understand where this logic break is.

Identify and loop through tables
    Create array from table '<= can be a function that returns an array - single responsibility
    Identify relevant Class
    Update Dictionary based on array '<= single responsibility,
        'parameters could be the array, the dictionary and
        'the instance of the class based on the interface

For example

Sub UpdateDictionary(ByVal Ary as Variant, ByRef Dict as Dictionary, ThisClass as IClass)
    For I = LBound(Ary, 1) To UBound(Ary, 1)
        If Dict.Exists(Ary(I, 1)) Then
            MsgBox "Entry already exists: " & Ary(I, 1)
        Else
            Set ThisClass = ThisClass.NewMe
            ThisClass.RelevantFillMethod(Ary(I, 1),Ary(I, 2))
            Dict.Add Ary(I, 1), ThisClass
        End If
    Next I
End Sub

Final comments

What I have suggested above may not be the most elegant - but it is a start. Making something generic is about being able to abstract to the relevant layer.


Check for whether a string contains any duplicate characters I am a new programmer and I want to know if I'm inheriting the right styles. I don't want to pick up any bad/amateur habits. Any advice would be much appreciated.

This is an isUnique function I wrote in C.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main()
{
    int x;
    char s1[] = "dog hi cobe!";
    x = isUnique(s1);
    printf("%d", x);
    return 0;
}

isUnique(char* s1)
{
    char holder;
    int i;
    int j;

    if(s1 == NULL)
        return -1;

    for(i = 0; i < (strlen(s1) -1); i++)
    {
        holder = s1[i];
        for(j = i; j < (strlen(s1)-1); j++)
        {
            if(holder == s1[j+1])
            {
                if(holder == ' ')
                    continue;
                else
                    return 0;
            }
        }
    }
    return 1;
}

And I'm going to say the run time complexity is \$O(n^2)\$, would I be correct?

Missing return type

This function declaration is missing the return type:

isUnique(char* s1)

Sure, the default return type is int in C,
but it would be better to just write it anyway for clarity.

Use boolean

C99 has a bool type with true and false values,
which you can get from this #include:

#include <stdbool.h>

It would be better to rewrite isUnique to return a bool.
Even though, this will mean giving up the special value -1 for the NULL input case. But that doesn't seem so bad. A name like "is*" is commonly expected to return a boolean. If you really need a function that can handle the NULL case, you could create a wrapper function:

/* returns -1 on NULL input, 0 if not unique, 1 if unique */
int checkUnique(char* s1) {
    if (s1 == NULL) return -1;
    return isUnique(s1);
}

Note that the bool return value of isUnique will be converted to int, so that false is 0 and true is 1.

Avoid repeated calculations

You call strlen(s1) twice:

for(i = 0; i < (strlen(s1) -1); i++)
{
    holder = s1[i];
    for(j = i; j < (strlen(s1)-1); j++)

It would be better to call it once, store in a variable and reuse.

Time complexity

As you suspected, the time complexity of your algorithm is \$O(n^2)\$:
all characters are compared with all other.

You could improve that to \$O(n)\$ by using extra storage, the size of the alphabet.
You could create an array of bool, representing if a character was already seen or not:

All values are initialized to false
For character in the input, check if it was seen

If it was seen, return false
If it was not seen, mark it now (set value in the bool array to true)

If the end of the string is reached, return true

Something like this:

bool isUnique(char * string) {
    int len = strlen(string);

    bool seen[256];
    memset(seen, false, sizeof(seen));

    int i;
    for (i = 0; i < len; i++) {
        char c = string[i];
        if (seen[c]) {
            return false;
        }
        seen[c] = true;
    }
    return true;
}

Printing the result

This will print a 0 or 1:

int x;
char s1[] = "dog hi cobe!";
x = isUnique(s1);
printf("%d", x);

It would be more informative to print the strings that was tested, and "false"/"true" instead of 0/1:

char s1[] = "dog hi cobe!";
printf("%s -> %s\n", s1, isUnique(s1) ? "true" : "false");

Usability

It would be more interesting if the program took the strings from the command line, instead of using hardcoded values, for example:

void printWithResult(char* string) {
    printf("%s -> %s\n", string, isUnique(string) ? "true" : "false");
}

int main(int argc, char ** argv) {
    int i;
    for (i = 1; i < argc; i++) {
        printWithResult(argv[i]);
    }
}


Get Filtered Data Range in a Table Purpose:
Given a Worksheet Table (ListObject) that may, or may not, have a filter currently applied to it, return a range consisting only of the filtered data.

Method:
Loop through each row in the table.
If row is visible, add it to a filteredRange via Union().
Return filteredRange.

Specific Areas of Interest:
Is the function name sufficiently intuitive?
Is there an easier way to achieve the desired results?
Are there edge cases I haven't accounted for?

Function:

Public Function GetFilteredTableRange(ByRef targetTable As ListObject, Optional ByVal includeHeaders As Boolean = False)
    '/ given a table, return a range object that contains only those rows which are visible
    '/ Do this by looping through tableRows, adding all visible rows to a unionRange

    Dim allDataRange As Range
    Set allDataRange = targetTable.DataBodyRange

    Dim filteredRange As Range
    Dim rowRange As Range

    If includeHeaders Then
        Set rowRange = targetTable.HeaderRowRange
        Set filteredRange = rowRange
    End If

    For Each rowRange In allDataRange.Rows()
        If rowRange.EntireRow.Hidden = False Then

            If filteredRange Is Nothing Then
                Set filteredRange = rowRange
            Else
                Set filteredRange = Union(filteredRange, rowRange)
            End If

        End If
    Next rowRange

    Set GetFilteredTableRange = filteredRange

End Function

Public Function GetFilteredTableRange(ByRef targetTable As ListObject, Optional ByVal includeHeaders As Boolean = False)
    '/ given a table, return a range object that contains only those rows which are visible
    '/ Do this by looping through tableRows, adding all visible rows to a unionRange

Newlines
are
cheap.

In Python, you always aim to keep your lines shorter than 80 characters long. I personally think it's a good practice to keep that size as a guideline in other languages as well. Your first line is 120, 1.5x my proposed maximum.

Added benefit of trying to stick with a maximum is it keeps you sharp. You're forced to think whether it's absolutely necessary for that line to be that long. Long lines are not fun to read, so try not to make them longer than necessary.

Public Function GetFilteredTableRange( _
    ByRef targetTable As ListObject, _
    Optional ByVal includeHeaders As Boolean = False _
)
    '/ Given a table, return a range object with only the visible rows.
    '/ Do this by looping through tableRows,
    '/ adding all visible rows to a unionRange.

That's how I'd do it, but I'm not up to par with VBA coding standards.


Time limit of a c program while calculating factorial of numbers in c I am solving a problem on calculation of factorial and the challenge is as follows!

You are asked to calculate factorials of some small positive integers.

Input

An integer t, 1<=t<=100, denoting the number of testcases, followed by t lines,
each containing a single integer n, 1<=n<=100.

Output

For each integer n given at input, display a line with the value of n!

My Code is giving me the correct solution but Time limit is exceeded which is 2 seconds:

The code is as follows:

#include<stdio.h>
#include<stdlib.h>
#include<math.h>
void factorial(int N)
{
    printf("\n\n");
    int q,i,j,t,d,z;
    float p=0.0;
    for(i=2;i<=N;i++)
    p=p+log10(i);
    d=(int)p+1;//No of terms in the factorial
    int *b;
    //initialization of an array
    b=(int *)malloc(d*sizeof(int));
    b[0]=1;
    for(i=1;i<d;i++)
    b[i]=0;
    //calculation of factorial
    p=0.0;
    for(j=2;j<=N;j++)
    {
        q=0;
        p=p+log10(j);
        z=(int)p+1;
        for(i=0;i<N;i++)
        {
           t=(b[i]*j)+q;
           q=t/10;
           b[i]=t%10;
        }
    }
    for(i=d-1;i>=0;i--)
    printf("%d",b[i]);

}
int main()
{
    int n,i,j;
    scanf("%d",&n);
    int *b;
    b=(int *)malloc(n*sizeof(int));
    for(i=0;i<n;i++)
    {
        scanf("%d",&b[i]);
    }
    for(i=0;i<n;i++)
    factorial(b[i]);
    return 0;
}

How can i make my program more efficient and produce the output in the given time limit? This challenge is from HackerEarth

The best solution is only to compute the factorials you need and only compute them once.

Read all the user input find the maximum value.
Generate the factorials for all values upto the max
Note: Saving them as you go:
Print the factorial values by looking up the result you generated in 2.

So lets look at the code:

I would not bother dynamically allocating the data storage.

    b=(int *)malloc(n*sizeof(int));

The question specifically limits the maximum value of n to 100.

    int b[101]; // should be suffecient

When reading values this should work (if you assume that the input is good).

        scanf("%d",&b[i]);

But the question states that the input is one value per line. Personally I would validate that there is one value per line.

Here you are calculating the factorial multiple times:

    for(i=0;i<n;i++)
       factorial(b[i]);  // Factorial is O(n)
                         // Thus this loop is O(n^2)

Technically you only need to call factorial once. If you calculate factorial for 'n' you need to calculate the factorial for 'n-1' (its how it is defined). If you store the numbers then you only need to look up the values to print it once they have been calculated.

It seems like you calculate this value each time.

    float p=0.0;
    for(i=2;i<=N;i++)
    p=p+log10(i);

The worst case scenario is that you need to calculate this for a value of 100. You only need to calculate this one for 100. Also you can calculate it off line and now it is a const expression.

This is then a const expression and thus you don't need to calculating it.

    b=(int *)malloc(d*sizeof(int));

But you do need 100 of them so you can store all intermediate values:

You don't need to loop over an array to initialize it:

    for(i=1;i<d;i++)
        b[i]=0;

You can initialize it already zero'd out. For dynamic memory use calloc() for automatic arrays use = {0};

So the above two can be combined into;

    const int d = <pre-calculated value>;
    int b[100][d] = {0};
    b[0][0] = 1;
    b[1][0] = 1;


Count consecutive ones in a binary list There is a list consisting of zeroes and ones. I want to find out the length of the longest streak of ones. Is there a better solution?

def consecutive_one(data):
   one_list = []
   size = 0
   for num in data:
       if num == 1:
           one_list.append(num)
       elif num == 0 and size < len(one_list):
           size = len(one_list)
           one_list = []

    return size

if __name__ == '__main__':
    data = [0, 1, 0, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 0]
    print(consecutive_one(data))

Both your and janos' implementations are broken:

data = [1, 0] * 10000
consecutive_one(data)
#>>> 140

This is because you don't always reset after seeing a 0. Going from janos', you should have

longest = 0
current = 0
for num in data:
    if num == 1:
        current += 1
    else:
        longest = max(longest, current)
        current = 0

return max(longest, current)

and equivalent for the original.

You'll find that this functionality is largely provided by itertools.groupby, though:

from itertools import groupby

def len_iter(items):
    return sum(1 for _ in items)

def consecutive_one(data):
    return max(len_iter(run) for val, run in groupby(data) if val)