Fluent interface for Logger I was trying to wrap a Logback logger in order to provide some handy methods and already defined default keys of the logged json output and I came up with something like this.

Do you spot any problem with it, or do you have any suggestion?

public class Log {

    private Logger logger = LoggerFactory.getLogger("jsonLogger");

    private String msg;
    private long start;
    private Map<String, Object> kvs;

    private Log(String msg) {
        this.msg = msg;
        this.start = System.currentTimeMillis();
        this.kvs = new HashMap<>();
    }

    public static Log msg(String msg) {
        return new Log(msg);
    }

    public Log kv(String key, Object value) {
        kvs.put(key, value);
        return this;
    }

    public void log() {
        kvs.put("elapsed", System.currentTimeMillis() - start);
        List<StructuredArgument> args = new ArrayList<>();
        for (String k : kvs.keySet()) {
            args.add(StructuredArguments.kv(k, kvs.get(k)));
        }
        logger.info(msg, args.toArray());
    }

}

In this way you can log just a message, custom key-values and have a way to keep track of the execution between a function:

// just a message
Log.msg("my message").log();

// message with custom kv or complex objects
Log.msg("my message").kv("foo", "bar").kv("arr", new String[]{"a", "b", "c"}).log();

// elapsed time
Log logger = Log.msg("my message").kv("foo", "bar");
Thread.sleep(1240);
logger.log();

i do not think this is really a good idea (sorry to say that)...

why is is not a good idea? it's breaking the concept of segregaton of concerns:

a Logger is responsible for logging
a Profiler is responsible for measuring execution time
a Logger is NOT responsible for keeping track of items (who is responsible for cleaning these entries?)

another issue i see is that you create a new logger whenever you want to write something into you log, i don't see any reason for that...

what would i suggest?

create a timesheet

public class Timesheet{

   private final Map<TimeStamp, KeyValue kv> entries;
   private final Logger logger;

    public Timesheet(Logger logger){
        this.logger = logger
        entries= ...
    }

    public Timesheet kv(Object key, Object value) {
        TimeStamp timestamp = new TimeStamp();
        entries.put(timestamp , new KeyValue(key, value));
        log.debug("adding key/value {}/{} at {}", key, value, timestamp );
        return this;
    }

    public Timesheet msg(Object msg) {
        logger.debug(msg);
        return this;
    }

    public Timesheet elapsed() {
        logger.debug("time elapsed {}", calculateTimeElapsed());
        return this;
    }

}

not all is implemented here, it should just girve you an idea of how you could do it...

Timesheet timesheet = new Timesheet(LoggerFactory.getLogger("jsonLogger")); //now availible for all classes
timesheet.msg("hello");
timesheet.kv("foo", "bar");
Thread.sleep(1234);
timesheet.elapsed();

some final words...

i think it is not a good idea of using KeyValue pairs - if you know what kind of entries these are, give them a proper name!

i think if you would have choosen a more compliant name (Log is really confusing) your idea would not have been so bad...


determining if a range is within array of ranges I have a List of a custom structure (myStruct) which has two ranges stored in it.
I'm trying to write a function where I can figure out if a range is completely enclosed in any of the ranges. Is this optimal and if not how can I improve it in terms of speed and memory efficiency?

class myStruct {
    private int start_one;
    private int stop_one;
    private int start_two;
    private int stop_two;

    public myStruct(int input_start_one, int input_stop_one, int input_start_two, int input_stop_two)
    {
        start_one = input_start_one;
        stop_one = input_stop_one;
        start_two = input_start_two;
        stop_two = input_stop_two;
    }
}

public void run()
{
    List<Struct> myStructs = new List<Struct>();
    myStructs.Add(new myStruct(86,724,725,1364));
    myStructs.Add(new myStruct(86,724,2068,2707));
    myStructs.Add(new myStruct(725,1364,2068,2707));

    if(is_in_range(88,100))
    {
        // in range
    }
}

public bool is_in_range(int in_start, int in_stop, BindingList<myStruct> input_list)
{
    myStruct current_item;
    for (int i = 0; i < input_list.Count; i++)
    {
        current_item = input_list[i];

        if (   (in_start >= current_item.start_one && in_stop <= current_item.stop_one)
            || (in_start >= current_item.start_two && in_stop <= current_item.stop_two))
                return true;
    }

    return false;
}

Just an idea

public struct Range
{
    public readonly int From;
    public readonly int To;

    public Range(int from, int to)
    {
        From = from;
        To = to;
    }

    public bool Contains(Range other)
    {
        return Contains(other.From, other.To);
    }

    public bool Contains(int from, int to)
    {
        return From <= from && To >= to;
    }
}

public class MyStruct
{
    public readonly Range Range1;
    public readonly Range Range2;

    public MyStruct(Range range1, Range range2)
    {
        Range1 = range1;
        Range2 = range2;
    }

    public MyStruct(int from1, int to1, int from2, int to2)
    {
        Range1 = new Range(from1, to1);
        Range2 = new Range(from2, to2);
    }

    public bool Contains(Range r)
    {
        return Range1.Contains(r) || Range2.Contains(r);
    }

    public bool Contains(int from, int to)
    {
        return Range1.Contains(from, to) || Range2.Contains(from, to);
    }
}

-

List<MyStruct> myStructs = new List<MyStruct>()
    {
        new MyStruct(86, 724, 725, 1364),
        new MyStruct(86, 724, 2068, 2707),
        new MyStruct(725, 1364, 2068, 2707)
    };

bool b = myStructs.Any(r => r.Contains(88, 100));


Converting DataTable to List of class I am using ExcelDataReader to import an Excel file to a dataset.

Example Excel table:

//ID     Name     Display Order    Active
//1      John          1             1

ID, DisplayOrder and Active columns are read as double, so I have to convert them to long, int and bool types respectively. I need to create a list of type Category from the DataTable of the DataSet.

Will this code perform well? Any suggestions for a faster conversion of DataTable to List of class?

var list = result.Tables["Categories"].AsEnumerable()
.Skip(1)
.Select(dr =>
        new Category
            {
                Id = Convert.ToInt64(dr.Field<double>("ID")),
                Name = dr.Field<string>("Name"),
                DisplayOrder = Convert.ToInt32(dr.Field<double>("Display Order")),
                IsActive= dr.Field<double>("Active") == 1 ? true : false
            }
        ).ToList();

I created an extension method for DataTable to convert them into a List<T>

public static class Helper
{
    /// <summary>
    /// Converts a DataTable to a list with generic objects
    /// </summary>
    /// <typeparam name="T">Generic object</typeparam>
    /// <param name="table">DataTable</param>
    /// <returns>List with generic objects</returns>
    public static List<T> DataTableToList<T>(this DataTable table) where T : class, new()
    {
        try
        {
            List<T> list = new List<T>();

            foreach (var row in table.AsEnumerable())
            {
                T obj = new T();

                foreach (var prop in obj.GetType().GetProperties())
                {
                    try
                    {
                        PropertyInfo propertyInfo = obj.GetType().GetProperty(prop.Name);
                        propertyInfo.SetValue(obj, Convert.ChangeType(row[prop.Name], propertyInfo.PropertyType), null);
                    }
                    catch
                    {
                        continue;
                    }
                }

                list.Add(obj);
            }

            return list;
        }
        catch
        {
            return null;
        }
    }
}

Example:

DataTable dtTable = GetEmployeeDataTable();
List<Employee> employeeList = dtTable.DataTableToList<Employee>();


Aggregate loans report without using Python standard aggregate or group functions I've written the following as a code assignment (Python 2.7). The task is to aggregate the loans csv file from the accounts department by (Network, Product, Month) with the total currency amounts, and counts, without using standard Python aggregate or group functions. I would like advice and suggestions on my current solution in order to learn from it going forward as my code was reviewed already.

I've used Python dict OrderedDict instead of normal Python dictionary to keep the current order of the records. I do realise that using OrderedDict comes at a cost, how else could I keep the order of the records that will be written to the Output Summary CSV.

Input Loans CSV

Output Loans Summary CSV

"""
Created on 20 August 2017

Loans Summary

Validation

@author: Peter Wilson
"""
import re
import csv
import argparse
import collections
from datetime import datetime

def csv_to_dict(loans_csv):
    """Convert loans csv to
    Python dictionary"""
    loans_dict = csv.DictReader(open(loans_csv))
    return loans_dict

def dict_to_agg_dict(loans_dict):
    """Aggregate loans by
    (Network, Product, Month)"""
    agg_dict = collections.OrderedDict()
    for row in loans_dict:
        network = re.sub(r"'", '', row['Network'])
        product = re.sub(r"'", '', row['Product'])
        # assumed month per year to be aggregated
        month = re.findall(r'([A-z]+?-\d{4})', row['Date'])[0]
        # assumed currency rounded to closet rounded number
        currency = float(row['Amount'])
        dict_key = (network, product, month)
        agg_dict.setdefault(dict_key, []).append(currency)
    return agg_dict

def agg_dict_to_lists(agg_dict):
    """Convert aggregated dictionary
    to Python list of lists"""
    summary_list = []
    for key, values in agg_dict.iteritems():
        values_list = list(key)
        currency_sum = sum(values)
        counts = len(values)
        values_list.insert(len(values_list), currency_sum)
        values_list.insert(len(values_list), counts)
        summary_list.append(values_list)
    return summary_list

def output_csv_summary(summary_list, output_csv_folder):
    """Write aggregated results
    into output csv"""
    current_date = datetime.today().strftime("{0}{1}{2}".format("%y", "%m", "%d"))
    csv_name = "{0}_{1}.csv".format("Output", current_date)
    output_csv = "{0}\\{1}".format(output_csv_folder, csv_name)
    print("Writing {0} to {1}".format(csv_name, output_csv_folder))
    csv_header = ["Network", "Product", "Month\Year", "Currency", "Count"]
    with open(output_csv, 'wb') as csvfile:
        csvwriter = csv.writer(csvfile)
        csvwriter.writerow(csv_header)
        for row in summary_list:
            try:
                csvwriter.writerow(row)
            except UnicodeError as e:
                print(e)

def validate_loans(loans_csv, output_csv_folder):
    """Validate loans summary from
    accounts with system"""
    loans_dict = csv_to_dict(loans_csv)
    agg_dict = dict_to_agg_dict(loans_dict)
    summary_list = agg_dict_to_lists(agg_dict)
    output_csv_summary(summary_list, output_csv_folder)

if __name__ == '__main__':
    parser = argparse.ArgumentParser(description='Validate accounting loans summary against system')
    parser.add_argument('--loans_csv', metavar='path', required=True,
                        help='path to input loans csv')
    parser.add_argument('--output_csv_folder', metavar='path', required=True,
                        help='path to output csv folder')
    args = parser.parse_args()
    validate_loans(loans_csv=args.loans_csv, output_csv_folder=args.output_csv_folder)

Try not to use name of data-structure in your variables. Check: Do you prefix variable names with an abbreviation of the variable types?. Python 2 doesn't support annotations but you can use comments to specify the types. On Python 3+ you can use function annotations and in Python 3.6+ you can use variable annotations as well.
Use with-statement while handling files: open(loans_csv).

  It is good practice to use the with keyword when dealing with file
  objects. The advantage is that the file is properly closed after its
  suite finishes, even if an exception is raised at some point.

Use simple str.split instead of regex r'([A-z]+?-\d{4})': row['Date'].split('-').
You can pass qotechar="'" to csv.DictReader and then you won't have to strip them manually.
To append an item at the end use list.append instead of vlist.insert(len(list), item).
The for-loop in agg_dict_to_lists can be reduced to:

for key, values in agg_dict.iteritems():
    values_list = list(key) + [sum(values), len(values)]
    summary_list.append(values_list)

If you're expecting currency to be decimal then use decimal module instead of plain floats. Decimals objects provides better floating-point arithmetic and alterable precision.
The format() is not required here: datetime.today().strftime("{0}{1}{2}".format("%y", "%m", "%d")). You could simply do: datetime.today().strftime('%y%m%d').
If you're opening the file in binary mode open(output_csv, 'wb') the make sure you're converting the data being written to bytes first by encoding individual items of the rows. This would otherwise not even work in Python 3 where you cannot mix bytes and str anymore.
print is not a function in Python 2 unless your import it using from __future__ import print_function. Hence, don't use it with parenthesis.


File access permission/deny function I've quickly made a function to deny or allow access to files that are only used thru Ajax calls.

Key Generator

/**
 * Generate a key for file access permission
 *
 * Function that returns a key to grant access to a PHP file
 * that will be consulted by an ajax call.
 *
 * @param str $salt Any given string to append to the session ID
 *
 * @return str Access Key
 */
public function file_access_key_generator($salt) {

    return sha1(session_id() . $salt);

}

Key Validation

/**
 * Match file access permission key
 *
 * Function to match the given key with the one generated.
 *
 * @param str $salt Any given string to append to the session ID
 * @param str $key  Key to match
 *
 * @return bollean
 */
public function file_access_key_check($salt, $key) {

    return ($this->file_access_key_generator($salt) == $key) ? true : false;

}

The goal, as mentioned, is to prevent direct access to files that should only be accessed thru an Ajax Call within the application.

The methodology implemented is as follows:

// Generate the key to pass with the Ajax Post variables
$check = $this->my_class->file_access_key_generator(basename(__FILE__, '.php'));

// Validating the key
if (isset($_POST['check']) &&
    $my_class->file_access_key_check(basename(__FILE__, '.php'), $_POST['check'])) {
     // do stuff...
} else {
     // friendly user message stating that the access to the file isn't autorized
}

Essentially, what is being done is to generate a key combining the file name and the session ID. Both must match otherwise the access isn't allowed.

Does this secures the access to the file or some considerations are to be made in order to actually secure the file that receives Ajax calls, thus preventing a direct browser access or the file being included within another one?

Short version:

You may trust your own PHP code, if you write it well.
You should not trust data received by cookies, $_GET or $_POST.
Attempting to distinguish data sent from your JavaScript application from malicious input relies on methods that are both hard and easily bypassed.

Long version:

First of all, do you really need the ? true : false?
The == operator already returns a boolean.

Forbidding direct access is a complex (impossible?) task.
The user might just watch the web traffic you send your own Ajax application, and then imitate the output your app would send.

Even if you could prevent that, it'd probably still be pointless - the user could just edit the JavaScript code in many ways (so many in fact that it's probably worthless to try to detect them) so the application would be manipulated into doing what the "attacker" wants.

If you are not concerned about the current application user but rather protecting your user against third parties, then using HTTPS might be the best option. Why re-implement what's already available and designed by people who are - let's face it - smarter than both of us combined (and with the added advantage that it's been around for some time, so its flaws have already had more time to be detected and fixed!)

At some point you are using __FILE__ to detect if the code is authorized? Does this mean the attacker can directly execute PHP code on your server?
If not, then your __FILE__ check is worthless.
If so, then your __FILE__ check is still worthless, as the attacker can just directly call the file functions bypassing your whole security system.

Remember this:

Users can manipulate all sent data. Cookies, $_GET and $_POST data can all be manipulated by the user.
All data you send your client-side application can be inspected by the user. See Firebug for a good example. If you send some_hash to the client, the user has ways to find that out.
You may, however, protect your user against third parties.
Code that is executed in the client-side can be manipulated. HTML, CSS and JavaScript can all be edited.
Fortunately, those edits are local. Meaning that an user can change the JavaScript that's executed on his computer, but fortunately not the JavaScript that's executed in someone else's computer - provided the server is well implemented.

Ajax code is typically executed client-side (I'm going to bet it's also true in your case), so all your JavaScript can be modified. Your "receive token" function can just be modified to include an alert function.

function receive_token() {
    var key = ajax_request_token();
    //Code injected by the user
    alert(key);
    //End code injected by the user
    return key;
}

The user could then proceed to use your key token to pretend he's the application.

Not sure what you mean by "the file being included within another one".


Maintainable way to concatenate strings in JS and vendor prefixes I am writing a jQuery plugin which, among other things, inserts a stylesheet in the head. Additionally, I also require vendor prefixes. A lot of values are derived from variables in the script, and I'm unsure how I can write this in a maintainable manner. Now I just list everything, but maybe there's a better way for vendor prefixing and overall structure of this script?

var bubble_in_ms = to_ms(args.bubble_in),
    opacity_out_ms = to_ms(args.opacity_out),
    text_in_ms = to_ms(args.text_in),
    bubble_color_rgb = rgbGenerator(args.bubble_color),
    bubble_css = "";

bubble_css += ".bubble {-webkit-animation: ";
bubble_css += "bubble-effect-in " + bubble_in_ms + "ms forwards,";
bubble_css += "bubble-effect-out " + opacity_out_ms + "ms ";
bubble_css += bubble_in_ms + "ms forwards;";
bubble_css += "animation: ";
bubble_css += "bubble-effect-in " + bubble_in_ms + "ms forwards,";
bubble_css += "bubble-effect-out " + opacity_out_ms + "ms ";
bubble_css += bubble_in_ms + "ms forwards;";
bubble_css += "background: ";
bubble_css += "rgba(" + bubble_color_rgb.r + "," + bubble_color_rgb.g + "," + bubble_color_rgb.b + ",";
bubble_css += args.bubble_opacity + ")";
bubble_css += "}";
bubble_css += ".bubble-wrap:after {-webkit-transition: ";
bubble_css += "opacity " + text_in_ms + "ms;";
bubble_css += "transition: ";
bubble_css += "opacity " + text_in_ms + "ms;";
bubble_css += "}";

Changing the many bubble_css += lines to a single long concatenation spanning multiple lines would be an improvement.

Even more improvement would be using a template framework,
for example moustache.

If you don't want to depend on a framework,
you could cook up something simple.
For example, use a single string with markups like {{ bubble_in_ms }},
and create a utility function that takes the template string and a dictionary of key-value pairs to replace in the template.


Redis work queue processor I frequently encounter the need for creating services that concurrently process messages from Redis queues. So I decided to share the knowledge I learned and help others easily bootstrap such a service. I would like a second opinion from code reviews of the library. Please do contribute if you feel like improving it!

Github repo: https://github.com/ikhoury/rstreamer

Sample usage

You create Worker classes that processes incoming messages from a queue.

public class SampleWorker implements Worker {

    @Override
    public void processSingleItem(String item) {
        System.out.println("Got item: " + item);
    }
}

You link workers to a task queue using a WorkSubscription:

List<Worker> workers = new ArrayList();
workers.add(new SampleWorker());
WorkSubscription subscription = new WorkSubscription("my:task:queue", workers);

You add your WorkSubscription to a SubscriptionManager that handles the lifecycle of all subscriptions.

subscriptionManager.addSubscription(subscription);
subscriptionManager.activateSubscriptions();

Background threads will spin up, continuously poll tasks, and process them using workers.

Really good library.

Here are my proposals:

1) Annotation mapping it would be really cool to use annotations (in spring boot application for example) like this:

@RstreamerProcessor
public class SampleWorker {

    @RstreamerListener(queue = "my:task:queue")
    public void processSingleItem(String item) {
        System.out.println("Got item: " + item);
    }
}

2) Add the library to maven central(publish artifact) or another repository and describe the usage on README.md

3) Consider to add Spring boot starter spring-boot-custom-starter

4) Add version compatibility table rstreamer to jedis and resilience4j.

5) It would be nice to have exception handling section with samples and description of errors and best practices to handle them.


99 simple asynchronous bottles on the wall I have some basic code which registers functions, with given variables, to be executed after a certain time has elapsed. I will roll this code into my software project when it is ready. If it works it works but since I've never done this before I am the Padawan and one of you out there, who's written code like this before, is the Jedi Master.
Here is my code:

#include <unistd.h>
#include <sys/time.h>
#include <stdlib.h>
#include <stdio.h>

typedef union EventArgument {
    int i;
    float f;
} event_argument;

typedef int (*event_function_ptr)(event_argument);

typedef struct EventList {
    event_function_ptr ptr;
    long time;
    event_argument arg;
    struct EventList* next;
} event_list;

event_list events_head;

int register_event (event_function_ptr event_funciton, long in_time, event_argument arg);

int event_print_int (event_argument arg) {printf("%i\n", arg.i);}
int event_print_float (event_argument arg) {printf("%f\n", arg.f);}
int event_kill (event_argument arg) {exit(arg.i);}

int event_99_bottles (event_argument arg) {
    if (arg.i == 0) {
        printf("no more bottles of beer on the wall!");
        event_argument arg_new;
        arg_new.i = -1;
        register_event (&event_kill, 5000, arg_new);
    } else {
        printf("%i bottles of beer on the wall. Take one down and pass it around,\n", arg.i);
        event_argument arg_new;
        arg_new.i = arg.i - 1;
        register_event (&event_99_bottles, 1000, arg_new);
    }

}

long current_time;

//loop from event_head until event.next is NULL or event.next.time strictly greater than current_time + in_time then malloc and insert in list.
int register_event (event_function_ptr ptr, long in_time, event_argument arg) {
    event_list* p = &events_head;
    event_list* q = malloc(sizeof(event_list));
    q->ptr = ptr;
    q->time = current_time + in_time;
    q->arg = arg;

    while (p->next != NULL && p->next->time <= q->time) {
        p = p->next;
    }
    q->next = p->next;
    p->next = q;
}

//loop through events in order and execute functions, then remove from list until event_head.next == NULL or event_head.next.time is greater than current_time.
int check_events (void) {
    while (events_head.next != NULL && events_head.next->time <= current_time) {
        event_list* p = events_head.next;
        p->ptr(p->arg);
        events_head.next = p->next;
        free(p);
    }
}

int update_time (void) {
    struct timeval time_val;
    gettimeofday(&time_val, 0);
    current_time = time_val.tv_usec / 1000 + time_val.tv_sec * 1000;
}

int main(void) {
    events_head.next = NULL;
    update_time();
    event_argument arg;
    arg.i = 99;
    register_event (&event_99_bottles, 20000, arg);
    arg.i = 512;
    register_event (&event_print_int, 5000, arg);
    arg.f = 3.1415;
    register_event (&event_print_float, 15000, arg);
    arg.i = 58008;
    register_event (&event_print_int, 7000, arg);

    while (1) {
        update_time();
        check_events();
        usleep(900000);
    }
}

Does this look good?
Is there anything missing?
Is there anything you think I should know now that will benefit me in future?

Here are some things that may help you improve your code.

Don't use usleep()

The usleep() function was part of the POSIX standard, but it was marked obsolete in 2001 and removed entirely from the 2009 version of the POSIX standard.  You can use nanosleep instead.

Make it clear when loops end

The code currently uses while (1) in main, but it doesn't really loop infinitely.  It's better, generally, to have the loop exit condition explicitly stated.  For example we could rewrite your main() loop:

while (events_head.next) {
    update_time();
    check_events();
    usleep(900000);
}

This makes it much more clear when and why the loop will end.

Return something useful from the subroutines

Many of the functions that claim to retun an int don't return anything at all, so they should be declared void instead.  Alternatively, if there's a value that could usefully be returned, modify the functions to return that instead.

Check return values and handle errors

The code calls malloc but never checks for error return values.  This is a serious problem that must be addressed.  If the sytem ever runs out of memory, the program will crash.

Consider re-using memory items

Each time an event is registered, new memory is allocated, and each time an event is processed, that memory is freed, even when, as in the case of event_99_bottles, the task re-registers itself.  One way to do this would be to have a reregister_event call.

Use a void * as the argument

Right now, if you decide to create a new type of event which uses a new type of argument, you'll have to recompile the whole code.  If instead of the EventArugment structure, the EventList contained a void *, then that part of the code would be able to remain the same, even if more elaborate data needed to be passed to specific events.  It would then be up to the called function to cast this to whatever it was expecting as input.

Consider using a priority queue

What you seem to be working toward is something like the event dispatcher for a generic task scheduling routine as might be used in a simple operating system.  Such task schedulers often use a priority queue to allow for higher and lower priority tasks.

Consider using an intelligent wait

The check_events() routine doesn't currently return anything useful, but it could.  If it traversed the list, checking for the next deadline and returned that time, then the code could simply sleep for that amount of time, very efficiently only waking up when there was work to do.

Eliminate global variables where practical

The code currently uses two global variables (current_time and events_head) that probably don't really need to be global.  Better would be to pass those as arguments to the functions that need them and/or putting them both inside an Events structure.  If you decide to keep either those (or an Events structure) outside of any function, consider making them static which limits them to file scope.  Generally speaking, it's good practice to make the scope of variables as small as practical.


Tip Calculator in javascript Update: revised code.

I work at a Starbucks and one of the things that screams at me week after week for automation is the tedious calculation of the tip distribution. So here is my attempt to provide a simple tool for this. (Previous attempt required special software and special instruction to explain how it works.)

It has fields for inputting the number of hours for each partner and the amount of money to be dispersed. Clicking "calculate" produces the output with blank lines to help orient the output with the input rows.

As a javascript noob, I feel certain I've done some stupids in here I can learn from.

    <title>Tip Calculator</title>

Tip Calculator

    Hours:

        <input type=button value="add row" onclick="add_row()">
        Dividend: <input id=dividend type=text size=5 maxlength=5>
        <input type=button value="calculate" onclick="calc()">

    &nbsp;

What you could do first is to separate out the logic that does the calculation from the logic that handles the UI. This makes it easier to update the math. Also, this makes the logic portable, should you scrap the UI code for something else, like if you move to use a framework.

Also, break up the logic to small functions that accept a predictable input. For instance, calculating shares is simply multiplying your inputs by your dividend - an array of input values and a number.

// In goes an array of hours and the dividend, out comes an array of shares
function getShares(hours, dividend){
  const sum = hours.reduce((c, v) => c + v);
  const rate = dividend / sum;
  const shares = hours.map(hour => hour * rate);
  return shares;
}

I also notice this strange E function which is just short for document.getElementById. There is document.querySelector and document.querySelectorAll that allows you to use CSS selectors to get a reference of your elements. Also, I'd avoid IDs since IDs should only appear once on the page. You can't use it for multiple elements of the same kind, for instance your inputs. Use classes instead.

For this case, I would tolerate the innerHTML usage. Otherwise, construct elements using document.createElement. Now to create repeated HTML, you can use array.map and array.join with template literals.

function renderResults(results){
  return results.map(result => `

${result.hours} x ${result.dividend} = ${result.share}

  `).join('');
}

For your HTML, while there are cases where the quotes on attributes are optional, it's best you put them in for consistency.


Pythonic way for double iteration (list all files with extensions) Given a base path and a list with extensions the task is to list all files:

Two of my solutions are:

from glob import glob
from os import path

EXTENSIONS = ['*.zip', '*.jar', '*.pdf']
DOC_PATH = '/path/to/files'

# Solution1:
files = []
for ext in EXTENSIONS:
    files.extend(glob(path.join(DOC_PATH, ext)))
# works but looks very clumsy

# Solution2:
files = reduce(lambda x,y: x+y,
               [glob(path.join(DOC_PATH, ext)) for ext in EXTENSIONS])
# Also functional but looks like a misuse of reduce

Have you got any other ideas?

If you only need to iterate over them (once) and not an actual list, you could use itertools.chain and glob.iglob:

files = chain(*(iglob(path.join(DOC_PATH, ext)) for ext in EXTENSIONS))

If you do need an actual list, you can further call list(files), of course.


Conditionally deleting rows in a worksheet I have used loops to remove lines that are doing various things, but all are deleting rows where certain criteria is met. Some are deleting less than certain numbers, deleting blanks, removing specific rows that contain specific things in columns, remove all the items that don't contain 4 specific parts. This works but it is slow and I'm sure someone can do this in a better way. Can anyone give me any advice?

Sub DeleteRandom()

Worksheets(2).Select
Application.ScreenUpdating = False
Application.DisplayStatusBar = False
ActiveSheet.DisplayPageBreaks = False
Dim r As Integer

'This part deletes out all the non essential items (basically like the filting does)

For r = Worksheets(2).UsedRange.Rows.Count To 1 Step -1
'Amount paid is more than 0
If Cells(r, "C") <= 0 Then
    Worksheets(2).Rows(r).EntireRow.Delete
'Compound Code field is not blank
    ElseIf Cells(r, "AE") = "" Then
        Worksheets(2).Rows(r).EntireRow.Delete
'Compound Code is 0 or 1
    ElseIf Cells(r, "AE") >= 2 Then
        Worksheets(2).Rows(r).EntireRow.Delete
'Claim type is for paid claims only
    ElseIf Cells(r, "L") <> "P" Then
        Worksheets(2).Rows(r).EntireRow.Delete
'Next 3 items remove CICS Claim Status that include CAC, OVR, or PWE
    ElseIf Cells(r, "J") = "CAC" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "J") = "OVR" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "J") = "PWE" Then
        Worksheets(2).Rows(r).EntireRow.Delete
'Next 9 items remove Drug Category Codes D, I, M, N, O, P, Q, R, or S
    ElseIf Cells(r, "S") = "D" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "I" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "M" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "N" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "O" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "P" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "Q" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "R" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "S") = "S" Then
        Worksheets(2).Rows(r).EntireRow.Delete
'Keeps only the Preauth = 0 rows
    ElseIf Cells(r, "N") <> 0 Then
        Worksheets(2).Rows(r).EntireRow.Delete
'Next 8 remove Group IDs that are not DS, GM, HP, or LP
    ElseIf Cells(r, "AD") = "940" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "MMPDMBA" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "MPXDMBA" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "PRODMBA" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "SCXFRGN" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "SPHDMBA" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "SPIDMBA" Then
        Worksheets(2).Rows(r).EntireRow.Delete
    ElseIf Cells(r, "AD") = "SPUDMBA" Then
        Worksheets(2).Rows(r).EntireRow.Delete
End If
Next

'This part adds the headers
Worksheets(2).Select
Range("A1").EntireRow.Insert
Worksheets(1).Select
'This part is picking only the specific headers we need (skipping all the other headers)
Range("D1:E1,G1:G1,Q1:Q1,U1:Y1,AK1:AK1,AW1:AW1,BD1:BD1,BK1:BK1,BR1:BS1,BW1:BW1,BY1:CA1,CE1:CE1,CG1:CG1,CI1:CJ1,CL1:CM1,CO1:CP1,CX1:CY1,DD1:DD1,HY1:HY1,KT1:KT1,LI1:LI1,MV1:MW1,NM1:NM1,PL1:PL1,PX1:PX1,QF1:QF1").Copy
 Worksheets(2).Select
Range("A1").Select
ActiveSheet.Paste
Application.ScreenUpdating = True
Application.DisplayStatusBar = True
ActiveSheet.DisplayPageBreaks = True
End Sub

You have fallen prey to a common temptation: writing a script. The problem with a script, is that it feels clunky and wrong, a bit like macro-recorder code does.

What is this code achieving?

Public Sub DeleteRandom()

Is it deleting random rows? Naming is hard. But it's worth it. Take the time to name things properly. Is it deleting non-essential rows? Then how about..

Public Sub DeleteNonEssentialRows()

So, back to the original question: what is this code achieving?

Turn off screen updating (and set calculation mode to manual?), for performance
Hide status bar and page breaks (for... what for?)
Iterate rows in the used range of Sheet2...
...delete unwanted rows
Copy headers to Sheet1
Restore status bar and page breaks
Restore screen updating

What's the copy headers to sheet1 doing there? Is that part of deleting non-essential rows? No! It's a completely separate concern, and one could argue it's an undesirable side-effect of calling the DeleteRandom procedure: if the calling code wants to copy headings to sheet1, let the calling code do it.

Make a procedure responsible for toggling screen updating, status bar and page breaks (and calculation mode?).. although I seriously question why you would want to hide the status bar at any time - this feels rather surprising, especially since you could instead be using it to convey some "Please wait..." message to the user so they know your code is running and Excel isn't frozen.

Make another procedure responsible for copying the headers from Sheet2 over to Sheet1, and get that concern out of this DeleteRandom procedure.

Now tackle the elephant in the room: make a function responsible for determining whether a row should be deleted or not.

Private Function IsNonEssentialRow(ByVal sheet As Worksheet, ByVal rowIndex As Long) As Boolean

End Function

Notice the sheet parameter: this function is going to work off a worksheet reference, and avoid another big problem with your current code: you're relying on Select and completely assume the active sheet is the sheet you need to be working with - and it might not be - or it could - or the user might have selected another sheet between two executing lines of code - regardless, your code relies on implicit references to Application.ActiveSheet, and that's bad.

Every time you call an unqualified Cells function, you make an assumption.

Keep a reference to your worksheet:

Dim sheet As Worksheet
Set sheet = ThisWorkbook.Worksheets("Sheet2")

...

If sheet.Cells(r, "C") <= 0 Then

Don't refer to them by index - the user is always free to reorder them!

Even better - sheets have a code name, which is basically a global-scope object reference that's ready to use - use it!

if Sheet2.Cells(r, "C") <= 0 Then

Select your sheet in the Project Explorer, and then hit F4 to access the properties toolwindow, and look for the CodeName property of your worksheet; give it a meaningful name, and then you can use that identifier in your VBA code.

Now, back to IsNonEssentialRow. The code should speak for itself - comments are good, but self-documenting code is better.

Dim amountPaid As Decimal
amountPaid = sheet.Cells(rowIndex, "C").Value

Dim compoundCode As Integer
compoundCode = sheet.Cells(rowIndex, "AE").Value

Dim claimType As String
claimType = sheet.Cells(rowIndex, "L").Value

Dim claimStatus As String
clainStatus = sheet.Cells(rowIndex, "J").Value

Dim categoryCode As String
categoryCode = sheet.Cells(rowIndex, "S").Value

Dim preAuth As String
preAuth = sheet.Cells(rowIndex, "N").Value

Dim groupId As String
groupId = sheet.Cells(rowIndex, "AD").Value

Now this looks like a bunch of very related values, doesn't it? How about encapsulating it into a type?

Private Type TRowItem
    AmountPaid As Decimal
    CompoundCode As String
    ClaimType As String
    ClaimStatus As String
    CategoryCode As String
    PreAuth As String
    GroupId As String
End Type

And now you can do this:

Dim item As TRowItem
item.AmountPaid = sheet.Cells(rowIndex, "C").Value
item.CompoundCode = sheet.Cells(rowIndex, "AE").Value
item.ClaimType = sheet.Cells(rowIndex, "L").Value
item.ClaimStatus = sheet.Cells(rowIndex, "J").Value
item.CategoryCode = sheet.Cells(rowIndex, "S").Value
item.PreAuth = sheet.Cells(rowIndex, "N").Value
item.GroupId = sheet.Cells(rowIndex, "AD").Value

Now that you've read everything you needed from the worksheet (once!), you're ready to start evaluating:

Dim result As Boolean

result = result Or item.AmountPaid <= 0
result = result Or item.CompoundCode = vbNullString
result = result Or CInt(item.CompoundCode) >= 2 'todo: define magic number 2
result = result Or item.ClaimType <> "P"
result = result Or StringMatchesAny(item.ClaimStatus,"CAC","OVR","PWE")
result = result Or StringMatchesAny(item.CategoryCode,"D","I","M","N","O","P","Q","R","S")
result = result Or item.PreAuth <> 0
result = result Or Not StringMatchesAny(item.GroupId,"DS","GM","HP","LP")

IsNonEssentialRow = result

For this to work you'll need a little helper function, StringMatchesAny, which returns True if a given string matches any of a specified bunch of values, and returns as soon as it finds a match:

Public Function StringMatchesAny(ByVal source As String, ParamArray values()) As Boolean

    Dim find As String, i As Integer, found As Boolean

    For i = LBound(values) To UBound(values)
        find = CStr(values(i))
        found = (source = find)
        If found Then Exit For
    Next

    StringMatchesAny = found

End Function

Then you could further refine the validation logic to return as soon as one of the rules evaluates to True, but I'll leave that to the reader.

With that validation logic out of the way, your DeleteNonEssentialRows procedure can focus on its task - the final code could look something like this (assuming some ToggleWaitMode(Optional ByVal wait As Boolean = True) procedure is implemented in another module):

Option Explicit
Private Type TRowItem
    AmountPaid As Decimal
    CompoundCode As String
    ClaimType As String
    ClaimStatus As String
    CategoryCode As String
    PreAuth As String
    GroupId As String
End Type

Public Sub DeleteNonEssentialRows()

    ToggleWaitMode

    Dim r As Long
    For r = Sheet2.UsedRange.Rows.Count To 1 Step -1
        If IsNonEssentialRow(Sheet2, r) Then
            Sheet2.Rows(r).EntireRow.Delete
        End If
    Next

    ToggleWaitMode False

End Sub

Private Function IsNonEssentialRow(ByVal sheet As Worksheet, ByVal rowIndex As Long) As Boolean

    Dim item As TRowItem
    item.AmountPaid = sheet.Cells(rowIndex, "C").Value
    item.CompoundCode = sheet.Cells(rowIndex, "AE").Value
    item.ClaimType = sheet.Cells(rowIndex, "L").Value
    item.ClaimStatus = sheet.Cells(rowIndex, "J").Value
    item.CategoryCode = sheet.Cells(rowIndex, "S").Value
    item.PreAuth = sheet.Cells(rowIndex, "N").Value
    item.GroupId = sheet.Cells(rowIndex, "AD").Value

    Dim result As Boolean

    result = result Or item.AmountPaid <= 0
    result = result Or item.CompoundCode = vbNullString
    result = result Or CInt(item.CompoundCode) >= 2 'todo: define magic number 2
    result = result Or item.ClaimType <> "P"
    result = result Or StringMatchesAny(item.ClaimStatus,"CAC","OVR","PWE")
    result = result Or StringMatchesAny(item.CategoryCode,"D","I","M","N","O","P","Q","R","S")
    result = result Or item.PreAuth <> 0
    result = result Or Not StringMatchesAny(item.GroupId,"DS","GM","HP","LP")

    IsNonEssentialRow = result

End Function

Or better yet, do like @Worker suggested and add the row to a range to delete in a single step.


Is there better way to shorten this Lambda expression's result selector? I'm new in LINQ and Lambda expression. I'm not sure is there a way to shorten the result on this o.z.cust.cust.cust.bintAccountNo.  As you can see below, the more table needed to join the longer the resultselector cust.cust.cust.  Is there better way to shorten this Lambda expression's result selector?

POCO

public class Tbl1
 {
        [Key]
        public long bintAccountNo{ get; set; }
 }

 public class Tbl2
 {
        [Key]
        public int id { get; set; }
        public long bintAccountNo{ get; set; }
 }

 public class Tbl3
 {
        [Key]
        public int id { get; set; }
        public long bintAccountNo{ get; set; }
 }

 public class Tbl4
 {
        [Key]
        public int id { get; set; }
        public long bintAccountNo{ get; set; }
        public int intPartner{ get; set; }
 }

 public class Tbl5
 {
        [Key]
        public int id { get; set; }
        public int intPartner{ get; set; }
 }

var b = (Tbl1
.GroupJoin(Tbl2, cust => cust.bintAccountNo, cust2 => cust2.bintAccountNo, (cust, cust2) => new  { cust = cust, cust2 = cust2 })
.SelectMany(cust => cust.cust2.DefaultIfEmpty(), (cust, cust2) => new { cust = cust.cust, cust2 = cust2 })
.GroupJoin(Tbl3, c => c.cust.bintAccountNo, cust3 => cust3.bintAccountNo, (cust, cust3) => new { cust = cust, cust3 = cust3 })
.SelectMany(c => c.cust3.DefaultIfEmpty(), (cust, cust3) => new { cust = cust.cust, cust3 })
.Join(Tbl4, c => c.cust.cust.bintAccountNo, bp => bp.bintAccountNo, (cust, bp) => new { cust = cust, bp = bp })
.Join(Tbl5, z => z.bp.intPartner, part => part.intPartner, (z, part) => new { z = z, part = part }))
.Select(o => new
  {
      bintAccountNo = o.z.cust.cust.cust.bintAccountNo
  }

Naming
Your entities / POCO classes have badly named members (assuming the entity names themselves aren't really named the way you've posted them), that break the C# naming conventions for public members - they should be PascalCase, ...and what's with the Hungarian notation? Also disemvoweling is never a great idea - call a table Table instead of Tbl, and we'll know you didn't mean Team-Based-Learning... whatever.
public class Table2
{
       [Key]
       public int Id { get; set; }
       public long AccountNumber{ get; set; }
}

Entities are NOT database tables; they MAP to database tables, and they're classes - so they should follow the naming conventions for C# classes. If they're mapped to tables/columns on SQL Server with different names, you should use the appropriate [Column("columnName")] attributes (or fluent API configurations) to specify the mapping.
Convention over Configuration
Entity Framework is able to infer a lot of things from the way you name things. Having a property called Id in an entity type, is automagically inferred to be that type's [Key] - hence, the explicit attribute is redundant / noise, and should be removed.
Tbl1 is breaking this convention. The [Key] field bintAccountNo should be called Id, and the table should be called Account - so when other related entities have a property named AccountId, Entity Framework knows that's a FK property.
Navigation Properties
One of the greatest things about EF is its ability to simplify the querying of complex relationships between entities, using navigation properties.
public class Account
{
    public long Id{ get; set; }
}

public class Table2
{
    public int Id { get; set; }
    public long AccountId { get; set; }

    public virtual Account Account { get; set; }
}

You mention in a comment that you find the generated T-SQL inefficient when you're using navigation properties; how exactly is it "inefficient"? It's verbose, yes, but that doesn't make it inefficient. Have you performance-profiled it? What's the bottleneck? Could you be missing indexes?

Your Query
I like that you're using the method syntax, but it's not clear what's being queries exactly, because the code you posted looks like it's querying the type itself:
var b = (Tbl1.GroupJoin(...));

One would expect something like this:
using (var context = new MyDbContext())
{
    var result = context.Tbl1s.GroupJoin(...));
}

GroupJoin
This is where the query syntax gets handy, see what exactly is a group join? on Stack Overflow:
Join syntax:
from p in Parent
join c in Child on p.Id equals c.Id
select new { p.Value, c.ChildValue }

GroupJoin syntax:
from p in Parent
join c in Child on p.Id equals c.Id into g
select new { Parent = p, Children = g }

And that calls GroupJoin() for you under the hood, and it's easier to mentally picture what your data looks like, I find.
I'm not quite clear on GroupJoin followed by SelectMany - why not just do a regular Join then?


LeetCode: Network Delay Time Bellman Ford C# https://leetcode.com/problems/network-delay-time/

  There are N network nodes, labelled 1 to N.

  Given times, a list of travel times as directed edges times[i] = (u,
  v, w), where u is the source node, v is the target node, and w is the
  time it takes for a signal to travel from source to target.

  Now, we send a signal from a certain node K. How long will it take for
  all nodes to receive the signal? If it is impossible, return -1.

Note:

  N will be in the range [1, 100].
  K will be in the range [1, N]. The
  length of times will be in the range [1, 6000].
  All edges times[i] = (u, v, w) will have 1 <= u, v <= N and 0 <= w <= 100.

Please review for performance

   [TestClass]
    public class NetworkDelayTimeTest
    {
        [TestMethod]
        public void ExampleTestBellmanFord()
        {
            int N = 4;
            int K = 2;
            int[][] times = { new[] { 2, 1, 1 }, new[] { 2, 3, 1 }, new[] { 3, 4, 1 } };
            NetworkDelayTimeBellmanFord bellmanFord = new NetworkDelayTimeBellmanFord();
            Assert.AreEqual(2, bellmanFord.NetworkDelayTime(times, N, K));
        }
    }

  public class NetworkDelayTimeBellmanFord
    {
        public int NetworkDelayTime(int[][] times, int N, int K)
        {
            var dist = Enumerable.Repeat(int.MaxValue, N + 1).ToList();
            dist[K] = 0;
            for (int i = 0; i < N; i++)
            {
                foreach (var e in times)
                {
                    int u = e[0];
                    int v = e[1];
                    int w = e[2];
                    if (dist[u] != int.MaxValue && dist[v] > dist[u] + w)
                    {
                        dist[v] = dist[u] + w;
                    }
                }
            }

            int maxWait = 0;
            for (int i = 1; i <= N; i++)
            {
                maxWait = Math.Max(maxWait, dist[i]);
            }

            if (maxWait == int.MaxValue)
            {
                return -1;
            }

            return maxWait;
        }
    }

You can optimize a bit by make it possible to step out of the first outer loop if the inner foreach-loop doesn't make any changes to dist in one iteration.

Wouldn't it be possible to test for maxWait whenever you update dist[v] in the foreach-loop:

{
    dist[v] = dist[u] + w;
    maxWait = Math.Max(maxWait, dist[v]);
}

This may og may not be an optimization - depending on the cost of comparing two integers potentially N * N times compared to the cost of an extra loop comparing them N-1 times?

You could make an early return in the find-max-loop:

       int maxWait = 0;
        for (int i = 1; i <= N; i++)
        {
          maxWait = Math.Max(maxWait, dist[i]);

          if (maxWait == int.MaxValue)
          {
            return -1;
          }
        }

or change it to:

int maxWait = dist.Skip(1).Max();
return maxWait < int.MaxValue ? maxWait : -1;


Writing Data.List's intersperse Learn You a Haskell shows the intersperse function:

ghci> intersperse '.' "MONKEY"
"M.O.N.K.E.Y"
ghci> intersperse 0 [1,2,3,4,5,6]
[1,0,2,0,3,0,4,0,5,0,6]

Note that the interspersed element only appears inside of the elements.

  list elem ... item ... list elem ..... item .. list elem

Originally, I tried to write this function in terms of a fold.

intersperse' :: [a] -> a -> [a]
intersperse' ys i = foldl (\acc x -> acc ++ [x] ++ [i]) [] ys

But, my approach failed since interspersed item was getting appended to the last element. Also, the lambda appended (++) to the acc each time, which isn't good.

So I decided to use pattern matching:

is' :: [a] -> a -> [a]
is' [] _ = []
is' (x:xs) i
  | null xs = [x]
  | otherwise = x : (i : (is' xs i))

Could the fold approach have worked? If so, how? Also, how's my is' implementation?

Could the fold approach have worked? If so, how?

intersperse' i = foldr (\x ys -> x : if null ys then [] else i : ys) []

In the fold, we cannot access the rest of the input, only the rest of the output. However, if and only if the input was empty, the output will be empty. So null ys happens to works here.

Interesting aspects:

I changed the argument order to match the standard intersperse. This happens to work better with the foldr-based definition. More importantely, I feel that intersperse is more often partially applied with an element than with a list, so the element should go first.
I use foldr instead of foldl to increase laziness and avoid the accumulator construction. Note how I don't need acc ++ [x] because I only add elements to the front of lists.
I make sure that intersperse' produces the first (:) cell before the null check.


Changing the foreground and background colors of substrings based on ANSI escape codes I am checking for particular keywords in a string, and then changing the foreground and background colors of substrings based on the keywords. Following is the snippet for checking the background code:

if(string.contains("[30;42m") || string.contains("[31;42m") || string.contains("[32;42m") ||
    string.contains("[33;42m") || string.contains("[34;42m") || string.contains("[35;42m")
    || string.contains("[36;42m") || string.contains("[37;42m")){
        //the parameter of indexOf and replaceAll will be one from the above
        //using [30;42m as an example
        index = string.indexOf("[30;42m");
        string = string.replaceAll("\\[30;42m", "");
        backgroundColor = Color.GREEN;
        //foreground is already set using separate conditions for 30,32,32...37m
     }

Is there any way that I can do something as I have shown above? Currently, I have separate conditions for each substring, which doesn't incorporates "DRY" principle. My code is working, but I am looking for a way to reduce these if statements.

Edit:
The if blocks are for setting the background and foreground colors. For example if I have "34;42" code, this means text foreground color is Blue and background color is Green, and so on.

You can do it a little more generic but I'm afraid you still have to test each.

I don't know if your case is always changing to green or not.
If not I suggest the following enum :

/**
 *
 * @author chillworld
 */
public enum ColorMapperEnum {
    GREEN("[30;42m",Color.GREEN) ,
    RED("[31;42m",Color.RED) ,
    BLACK("[32;42m", Color.BLACK) ,
    BLUE("[33;42m", Color.BLUE); // and so on

    private String textToFind;
    private Color targetColor;

    private ColorMapperEnum(String textToFind, Color targetColor) {
        this.textToFind = textToFind;
        this.targetColor = targetColor;
    }

    public String getTextToFind() {
        return textToFind;
    }

    public Color getTargetColor() {
        return targetColor;
    }
}

Now you can do a whole lot more generic in your java code :

for (ColorMapperEnum colorMapper : ColorMapperEnum.values()) {
    if(string.contains(colorMapper.getTextToFind()){
        index = string.indexOf(colorMapper.getTextToFind());
        string = string.replaceAll("\\" + colorMapper.getTextToFind(), "");
        backgroundColor = colorMapper.getTextToFind();
    }
 }

I did use the same naming as yo but I advice to take better naming.
String string => very bad, for what stands it?

Now I don't know if there could be a case that multiple conditions are true and what there must be done when multiple conditions are true.

An enum values() has always the same result in the same order.
Use that in your advantage when tackling that problem.


Telling me what lesson I have tomorrow and emailing me the results This program tells me what lessons I have tomorrow then emails them to me.

This program works fine, but I was wondering whether there is a way to simplify the script or make it more efficient. I have removed my email and password and who it is to for privacy, but it does work.

import smtplib
import time
import datetime
from datetime import date

days = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"]
English = ["MondayA", "WednesdayA", "ThursdayA", "FridayA", "TuesdayB", "WednesdayB", "ThursdayB"]
Maths = ["MondayA", "TuesdayA", "WednesdayA", "ThursdayA", "FridayA", "TuesdayA", "FridayB"]
Bioligy = []    #I will fill the following three
Physics = []
Chemistry = []
Computing = ["TuesdayA", "WednesdayA", "ThursdayA", "WednesdayB", "ThursdayB"]
History = ["MondayA", "TuesdayA", "MondayB", "TuesdayB", "WednesdayB"]
PE = ["ThursdayA", "WednesdayB", "FridayB"]
BaV = ["TuesdayA", "FridayB"]
French = ["MondayA", "FridayA", "MondayB", "TuesdayB", "FridayB"]
Course42 = ["MondayB"]
lesson = []
Comma = (", ")

day1 = (datetime.datetime.today().weekday())
day = (days[day1 + 1])

def WeekA(a):
    Oday = (a + "A")

    if Oday in English:
        lesson.append("English")

    if Oday in Maths:
        lesson.append("Maths")

    if Oday in Bioligy:
        lesson.append("Bioligy")

    if Oday in Physics:
        lesson.append("Physics")

    if Oday in Chemistry:
        lesson.append("Chemistry")

    if Oday in Computing:
        lesson.append("Computing")

    if Oday in History:
        lesson.append("History")

    if Oday in PE:
        lesson.append("PE")

    if Oday in BaV:
        lesson.append("B&V")

    if Oday in French:
        lesson.append("French")

    if Oday in Course42:
        lesson.append("Course42")

def WeekB(b):
    Oday = (b + "B")

    if Oday in English:
        lesson.append("English")

    if Oday in Maths:
        lesson.append("Maths")

    if Oday in Bioligy:
        lesson.append("Bioligy")

    if Oday in Physics:
        lesson.append("Physics")

    if Oday in Chemistry:
        lesson.append("Chemistry")

    if Oday in Computing:
        lesson.append("Computing")

    if Oday in History:
        lesson.append("History")

    if Oday in PE:
        lesson.append("PE")

    if Oday in BaV:
        lesson.append("B&V")

    if Oday in French:
        lesson.append("French")

    if Oday in Course42:
        lesson.append("Course42")

WeekA(day)

content = ("You have " + Comma.join(lesson) + ".")

mail = smtplib.SMTP('smtp.gmail.com',587)

mail.ehlo()

mail.starttls()

mail.login('*email*','*password*')

mail.sendmail('*email*','*email*', content)

mail.close()

I would change the data organization to be the opposite way, to have a dict mapping to the two week types containing a dict with a mapping from the weekday to the lessons:

timetable = {"A": {"Monday": ["English", "Maths", ...],
                   "Tuesday": ["Maths", "Computing", ...],
                   ...},
             "B": {"Monday": ["History", ...],
                   ...}}

By the way, it is spelled Biology, not Bioligy.

Then you can just use it like this:

import smtplib
import datetime

TIMETABLE = {"A": [["English", "Maths", ...],
                   ["Maths", "Computing", ...],
                   ...],
             "B": [["History", ...],
                   ...]}

SEMESTER_START = datetime.datetime(2016, 9, 1)

def calculate_week_type(date, start_date):
    """Even weeks since start_date are "A" weeks, uneven are "B" weeks"""
    return "A" if ((date-start_date).days / 7) % 2 == 0 else "B"

def lessons(timetable, start_date, date):
    week_type = calculate_week_type(date, start_date)
    return timetable[week_type][date.weekday()]

def mail_lessons(lessons, to_email='*email*'):
    content = "You have {} today.".format(", ".join(lessons))
    mail = smtplib.SMTP('smtp.gmail.com', 587)
    mail.ehlo()
    mail.starttls()
    mail.login('*email*', '*password*')
    mail.sendmail('*email*', to_email, content)
    mail.close()

if __name__ == '__main__':
    today = datetime.datetime.today()
    mail_lessons(lessons(TIMETABLE, SEMESTER_START, today))

I put the different responsibilities into separate functions, got rid of most of the global variables, and fixed some small differences to PEP8, python's official style-guide. I used a format string, because it is in general faster than string addition (it is usually more readable as well).

I also made the mapping now using numerical weekdays as indices into the internal list of lists, because this is what datetime returns. This way there is no need to translate between the two.


Team Fortress 2 item price lookup using Trade.tf developer API and Steam WebAPI I've made a nice little Python program using the requests module using the Trade.tf Developer API and the Steam WebAPI allowing the user to look up the price of Team Fortress 2 items through Trade.tf's spreadsheet.

For those of you that may not know, Team Fortress 2 is a free-to-play team-based shooter game which also features quite a large e-economy for in-game items. Sites such as Backpack.tf and Trade.tf exist to show pricing of items in terms of various currency-like items such as refined metal (a crafting ingredient in-game) and keys (used to open supply crates which can unlock various items within them).

I know there's definitely something that can be improved in this program, but I'm not fully sure on how I can accomplish this. Feel free to make any suggestions for it.

import json
import requests
import time

with open('keys.json', 'r') as f:
    keys = json.load(f)
    schema = requests.get('http://api.steampowered.com/IEconItems_440/GetSchema/v0001/?key=' + keys['steam'])
    spreadsheet = requests.get('http://www.trade.tf/api/spreadsheet.json?key=' + keys['tradetf'])

qualities = {"uncraft"    : -1,
             "genuine"    : 1,
             "vintage"    : 3,
             "unique"     : 6,
             "strange"    : 11,
             "haunted"    : 13} # all qualities currently tracked by Trade.TF

def get_item_defindex_from_name(name):
    for items in schema.json()["result"]["items"]:
        if items["name"] == name:
            defindex = items["defindex"]
    if defindex:
        return defindex
    else:
        print("Invalid item.")

def get_quality_id_from_name(name):
    return qualities[name]

def get_price_unit(unit):
    return {
    "r": " refined",
    "k": " keys",
    "b": " buds"}.get(unit, " ")

def get_tradetf_price(item, quality):
    price = spreadsheet.json()["items"][str(item)][str(quality)]["regular"]["hi"]
    unit  = get_price_unit(spreadsheet.json()["items"][str(item)][str(quality)]["regular"]["unit"])
    return str(price) + unit

def get_input(dialog, validation, error_message):
    while True:
        answer = input(dialog)
        if validation(answer):
            return answer
        else:
            print(error_message)

def main():
    try:
        while True:
            try:
                item_name = str(input("Enter item name: "))
                item_ = get_item_defindex_from_name(item_name)
                break
            except UnboundLocalError:
                print("Invalid item.")

        quality = get_input(
            "Enter item quality: ",
            lambda quality: quality.lower() in qualities,
            "Quality not tracked by Trade.tf."
        )
        quality_ = get_quality_id_from_name(quality.lower())

        print("\n", quality, item_name, "price: ", get_tradetf_price(item_, quality_))
    except KeyError:
        print("Item not currently tracked by Trade.tf.")

if __name__ == "__main__":
    main()
    print("Shutting down...")
    time.sleep(3)

Hah, this is a fun one cause I play a lot of Tf2. So let's see what could be improved here.

I'll start with you naming. You don't need to use the _from_ naming convention. ie.

def get_item_defindex_from_name(name):

get_item_defindex_from_name(item_name)

You already have the name there, it's clear that you need a name to get the defindex from, so remove that part and save yourself from a 5 word function name

get_item_defindex(item_name)

You should also use constants. qualities is essentially a constant but you haven't signalled it as one. To make it clear it is, write the name in UPPER_SNAKE_CASE (QUALITIES) and define it right after your imports, before running any actual code. I would do the same with your price units, it's not really clear why you treated them differently.

Your with block is a bit misplaced. If you definitely want to load the schema and spreadsheet as part of the global namespace to be accessed in every function, then you should still put them in the if __name__ == "__main__" block. if blocks don't limit scope the way functions do, so they'll still be globally available. You also never use the spreadsheet or the schema without calling .json() on them. Couldn't you just do this here? If you did you could probably save significant time not having to run that function every time. In fact, you only need items out of spreadsheet and ["result"]["items"] out of schema. If you don't need to maintain some sort of connection to the results (which it doesn't seem like you do) then you can just store the relevant values from these dicts and ignore the rest.

In get_item_defindex, you're looping over all the items to try find one with a matching name. Is there any reason to keep looking after you've found a value? I'm assuming not, so in that case you should return as soon as you find it to save time:

def get_item_defindex_from_name(name):
    for items in schema.json()["result"]["items"]:
        if items["name"] == name:
            return items["defindex"]
    print("Invalid item.")

If you specifically need to get the last matching name for some reason, then you should loop over the schema backwards:

def get_item_defindex_from_name(name):
    for items in reversed(schema.json()["result"]["items"]):
        if items["name"] == name:
            return items["defindex"]
    print("Invalid item.")

get_quality_id_from_name seems like a redundant function to me. If it's just going to be a dictionary lookup, it's easier to use that instead of a function. If it included getting the user input then it could be worth being a function. Often single lines of code are only worth being a function if they're particularly complex and worth abstracting or if they're called a lot and it saves repetitive code. Neither is true here. I'd say the same for price unit.

In get_tradetf_price you can make it readable and slightly more performant to grab the item up front, before you get the price and unit:

def get_tradetf_price(item, quality):
    item_data = spreadsheet.json()["items"][str(item)][str(quality)]["regular"]
    price = item_data["hi"]
    unit  = get_price_unit(item_data["unit"])
    return str(price) + unit

Your get_input function is very confusing. It's a good abstraction mostly, but the validation approach is overcomplicated, especially for what you need here. Instead of passing a lambda to test the input, just pass a list of valid values and test if the input is in there, like so:

def get_input(dialog, valid_inputs, error_message):
    while True:
        answer = input(dialog).lower()
        if answer in valid_inputs:
            return answer
        else:
            print(error_message)

You can then call it with a much simpler line:

    quality = get_input("Enter item quality: ", QUALITIES,
                        "Quality not tracked by Trade.tf.")

Also note I added the .lower() to when you call input. When accepting input try to sanitise it immediately. You don't care about the case so lowercase it as soon as you accept the input.

Note that if you can store the dictionary of items from the schema you can then reuse this for your item_name request at the start:

    item_name = get_input("Enter item name: ", schema_items,
                          "Invalid item.")

And lastly, your try block is a bit long. You clearly intend it to catch errors from get_tradetf_price but it's going to swallow any KeyError in your whole main block. Make try blocks as short as possible:

def main():
    item_name = get_input("Enter item name: ", schema_items,
                          "Invalid item.")
    item_id = get_item_defindex(item_name)

    quality = get_input("Enter item quality: ", QUALITIES,
                        "Quality not tracked by Trade.tf.")
    quality_id = get_quality_id_from_name(quality.lower())

    try:
        print("\n {} {} price: {}".format(quality, item_name,
                                          get_tradetf_price(item_id, quality_id))
    except KeyError:
        print("Item not currently tracked by Trade.tf.")


Many nested session verification conditions Can someone give me advice on cleaning this code? It's more messy then I expected. I got like 10-15 more ifs to be added.

I've thought of adding the error messsages in methods and then just check if method returns true, or false it'd throw error..

    if( isset($_SESSION["access_token"], $_SESSION["user_email"], $_SESSION["username"]) ){
    if($GoogleAccess->does_account_exist){
        if(!$GoogleAccess->is_g_acc_banned()){
            if(!$GoogleAccess->is_ip_banned()){
                if(!$GoogleAccess->is_ip_blacklisted($blacklist_ips)){
                    if($GoogleAccess->is_user_acc_verified()){
                        header("location: ../member.php");
                    }else{
                        $error = "Please verify your account";
                    }
                }else{
                    $error = "This IP's blacklisted.";
                }
            }else{
                $error = "Your IP's been banned";
            }

        }else{
            $error = "This account has been banned";
        }
    }else{
        //mean doesn't have account. Register user and send verification to email
    }

This is indeed quite messy. It's really hard to see which else ends which if.

Just invert the if and return early:

if( !isset($_SESSION["access_token"], $_SESSION["user_email"], $_SESSION["username"]) ){
    return false; // mean doesn't have account. Register user and send verification to email
}

if(!$GoogleAccess->does_account_exist){
    return "This account has been banned";
}

if($GoogleAccess->is_g_acc_banned()){
    return "Your IP's been banned";
}

...

Alternatively, if you don't have a function, you could also phrase it with if-elses:

if( !isset($_SESSION["access_token"], $_SESSION["user_email"], $_SESSION["username"]) ){
    return false; // mean doesn't have account. Register user and send verification to email
} else if(!$GoogleAccess->does_account_exist){
    $error = "This account has been banned";
} else if ($GoogleAccess->is_g_acc_banned()) {
    $error = "Your IP's been banned";
}

...

Personally, I prefer the first approach, as it's a lot more readable.

I'm not familiar with google access, but it seems odd that you have to manually check all the access denied reasons. I would expect that you call their API, and it either logs a user in, or returns the reason why that was not possible. Did you check if something like this exists? It would severely simplify your code.


Inverse mapping of a dictionary For the below question picked from here:

  Write make_inverse_dict(d) that returns a new dictionary with the
  'inverse' mapping. The 'inverse' mapping of a dictionary d is a new
  dictionary that maps each of d's values to all keys in d that
  mapped to it. For instance:

>>> d1 = {'call': 3, 'me': 2, 'maybe': 3}
>>> d2 = make_inverse_dict(d1)
>>> d2  # note that we know nothing about the order of dictionaries
{3: ('maybe', 'call'), 2: ('me',)}

  The ordering of the tuple of keys doesn't matter, i.e., d2 could
  have instead been {3: ('call', 'maybe'), 2: ('me',)}.

Below is the solution:

d1 = {'me': 2, 'call': 3, 'may be': 3}

def make_inverse_dict(d1):
    d2 = {}
    for key in d1:
        if d2.get(d1[key]) == None:
           d2[d1[key]] = (key,)
        else:
            d2[d1[key]] += (key,)
    return d2

d2 = make_inverse_dict(d1)

Additionally this is the recursive solution:

d1 = {'me': 2, 'call': 3, 'may be': 3}
def isEmpty(dictionary):
    for element in dictionary:
        if element:
            return False
    return True

def make_inverse_dict_recur(d1):
    temp = {}
    def make_inverse_dict():
        if not isEmpty(d1):
            tup = d1.popitem()
            if temp.get(tup[1]) == None:
                temp[tup[1]] = (tup[0], )
            else:
                temp[tup[1]] += (tup[0], )
            make_inverse_dict()
    make_inverse_dict()
    return temp

d2 = make_inverse_dict_recur(d1.copy())

Can we improve these solutions?

I think the recursive approach, using a nested function, is a bit awkward. I tend to avoid reliance on closures ("explicit is better than implicit", after all!) and use explicit parameters instead.

To create a temp dictionary only on the top level call, I would use:

def recur_func(..., temp=None):
    if temp is None:
        temp = {}
    ...
    recur_func(..., temp)

(if you're wondering why not temp={} in the parameter list, see "“Least Astonishment” in Python: The Mutable Default Argument")

Rather than tup = d1.popitem() then indexing into the tuple, I would use key, val = d1.popitem(); this improves the readability of subsequent lines, as e.g. key is much more informative than tup[0]. You can also use dict.setdefault (or, as pointed out in Günther's answer, collections.defaultdict) to neaten up the logic:

key, val = d1.popitem()
temp.setdefault(val, []).append(key)

That just leaves factoring out the nested function, and making make_inverse_dict_recur directly recursive:

def make_inverse_dict_recur(d1, temp=None):
    if temp is None:
        temp = {}
    if not isEmpty(d1):  # why not just 'if d1:'?
        key, val = d1.popitem()
        temp.setdefault(val, []).append(key)
        make_inverse_dict_recur(d1, temp)
    return temp

Again, though, contrast that with the iterative version:

def make_inverse_dict_iter(d1):
    temp = {}
    for key, val in d1.items():
        temp.setdefault(val, []).append(key)
    return temp

This has the neat advantage of not mutating d1; generally, Python functions should either mutate their arguments or return a new object, not both. I suspect that this would be (at best) tricky to achieve with a recursive approach, though.


2 way communication Python socket I'm trying to create a 2 way communication socket in python. The socket will listen for connections from the client until it gets the data formatted in string like this: 'PHONENUMBER|STATUS'. I will need to split that and convert it to json which will be sent via http request to a web api. When the api receives, it will handle the request and then it will return a status in json format which I will convert to string to return it to the client as string. I would like to have a professional opinion about my code and any additional ideas to what I would like to achieve would be appreciated.

import socket
import json
import requests
import httplib

HOST= '127.0.0.1'
PORT= 80

s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

#Bind to adress

s.bind((HOST,PORT))
print ("Socket bound.")

#Start listening
s.listen(10)
print ("Socket Listening")

conn, addr = s.accept()
url = 'http://www.google.com'
while True:
      data = conn.recv(1024)

      dictionary = dict(item.split("|") for item in data .split(";"))

      json = json.dumps(dictionary, ensure_ascii=False)

      r = requests.get(url, data=json)

 print r.text

This code does not compile, and has many bugs. Can you fix the errors in it?

url- = is a syntax error.

I don't recommend calling a variable split when you are using a method split, it's confusing.

You did not use that variable named split.

You did not define a variable message.

print 'r.text' should I suppose be print r.text.

I'm not sure about data = conn.recv(1024), what will happen if a request is longer than 1024 bytes?

This code needs to be debugged, it is not ready for code review. But it seems like a nice idea, and structurally okay, a good start.  I could help you debug it and get it working if you like.


Compute conditional median of PANDAS dataframe I am new to Python/Pandas.
Consider the following code:

import pandas as pd
import numpy as np

df = pd.DataFrame({'Time': [0.0, 1.0, 2.0, 0.0, 1.0, 2.0, 0.0, 2.0, 0.0, 1.0, 2.0],
                   'Id': [1, 1, 1, 2, 2, 2, 3, 3, 4, 4, 4],
                   'A': [10, 15, np.NaN, 11, 16, 25, 10, 15, 9, 14, 19]})
print(df)

Output:

       A  Id  Time
0   10.0   1   0.0
1   15.0   1   1.0
2    NaN   1   2.0
3   11.0   2   0.0
4   16.0   2   1.0
5   25.0   2   2.0
6   10.0   3   0.0
7   15.0   3   2.0
8    9.0   4   0.0
9   14.0   4   1.0
10  19.0   4   2.0

I want to add a column Feature_1 which, for each row of the dataframe, compute the median of column A for ALL the values which have the same Time value. This can be done as follows:

df['Feature_1'] = df.groupby('Time')['A'].transform(np.median)
print(df)

Output:

       A  Id  Time  Feature_1
0   10.0   1   0.0       10.0
1   15.0   1   1.0       15.0
2    NaN   1   2.0       19.0
3   11.0   2   0.0       10.0
4   16.0   2   1.0       15.0
5   25.0   2   2.0       19.0
6   10.0   3   0.0       10.0
7   15.0   3   2.0       19.0
8    9.0   4   0.0       10.0
9   14.0   4   1.0       15.0
10  19.0   4   2.0       19.0

My problem is now to compute another feature, Feature_2, which for each row of the dataframe, compute the median of column A for OTHER values which have the same Time value. I was not able to vectorize this, so my solution with a for loop:

df['feature_2'] = np.NaN

for i in range(len(df)):

    current_Id = df.Id[i]
    current_time = df.Time[i]

    idx = (df.Time == current_time) & (df.Id != current_Id)

    if idx.any():
        df['feature_2'][i] = df.A[idx].median()

print(df)

Output:

       A  Id  Time  Feature_1  Feature_2
0   10.0   1   0.0       10.0       10.0
1   15.0   1   1.0       15.0       15.0
2    NaN   1   2.0       19.0       19.0
3   11.0   2   0.0       10.0       10.0
4   16.0   2   1.0       15.0       14.5
5   25.0   2   2.0       19.0       17.0
6   10.0   3   0.0       10.0       10.0
7   15.0   3   2.0       19.0       22.0
8    9.0   4   0.0       10.0       10.0
9   14.0   4   1.0       15.0       15.5
10  19.0   4   2.0       19.0       20.0

This is working but it is very slow as my dataframe has 1 million rows (but only four different IDs).

Is it possible to vectorize the creation of Feature_2 ?

I hope, I am clear enough. Live code can be found here.

So, you want to get the medians of the groups by removing each value from the group in turn:

        group  =>  individual removal of values

          NaN      [  ]   NaN   NaN   NaN
         25.0  =>  25.0  [  ]  25.0  25.0
         15.0      15.0  15.0  [  ]  15.0
         19.0      19.0  19.0  19.0  [  ]

median   19.0      19.0  17.0  22.0  20.0

An other way of doing, beside manually reconstructing the group without the current value for each value, is to build the above intermediate matrix and ask for the median on each column. This will return a Series of length the length of the group, which is supported by SeriesGroupBy.transform.

The steps to get the desired result are:

build the matrix by repeating the input group as many time as its length;
fill the diagonal of the matrix with NaNs;
ask for the median by row/column depending on how you built the matrix.

The function that can be fed to transform may look like:

def median_without_element(group):
    matrix = pd.DataFrame([group] * len(group))
    np.fill_diagonal(matrix.values, np.NaN)
    return matrix.median(axis=1)

An other advantage of this approach is that you are able to reuse the same groups of elements and so cut on the need to recompute them again and again:

import numpy as np
import pandas as pd

def median_without_element(group):
    matrix = pd.DataFrame([group] * len(group))
    np.fill_diagonal(matrix.values, np.NaN)
    return matrix.median(axis=1)

def compute_medians(dataframe, groups_column='Time', values_column='A'):
    groups = dataframe.groupby(groups_column)[values_column]
    dataframe['Feature_1'] = groups.transform(np.median)
    dataframe['Feature_2'] = groups.transform(median_without_element)

if __name__ == '__main__':
    df = pd.DataFrame({
            'Time': [0.0, 1.0, 2.0, 0.0, 1.0, 2.0, 0.0, 2.0, 0.0, 1.0, 2.0],
            'Id': [1, 1, 1, 2, 2, 2, 3, 3, 4, 4, 4],
            'A': [10, 15, np.NaN, 11, 16, 25, 10, 15, 9, 14, 19],
    })
    compute_medians(df)
    print(df)


Queue over resizable array implementation I study data structures on coursera's course, and there is an extra exercise to create a Queue data structure.

I created it:

class Queue {

  Integer[] data;
  int head, tail;

  public Queue() {
    data = new Integer[2];
    head = 0;
    tail = 0;
  }

  public int size() {
    return tail - head;
  }

  public boolean isEmpty() {
    return size() == 0;
  }

  private void realign() {
    java.util.Arrays.sort(data, new java.util.Comparator<Object>() {
      public int compare(Object o1, Object o2) {
        if (o1 == null) return 1;
        else if(o2 == null) return -1;
        else return 0;
      }
    });
    tail -= head;
    head = 0;
  }

  private void resize() {
    if (tail == data.length && size() != data.length) {
      realign();
    }
    if (data.length == size()) {
      int newLength = data.length * 2;
      //Duplication
      Integer[] newData = new Integer[newLength];
      System.arraycopy(data, 0, newData, 0, data.length);
      data = newData;
    } else if (size() == data.length / 4 && size() != 0) {
      int newLength = data.length / 2;
      //Duplication
      Integer[] newData = new Integer[newLength];
      System.arraycopy(data, 0, newData, 0, data.length);
      data = newData;
    }
  }

  public void enqueue(Integer item) {
    if (item == null) { throw new NullPointerException(); }
    resize();
    data[tail] = item;
    tail++;
    return;
  }

  public int dequeue() {
    if (isEmpty()) { throw new java.util.NoSuchElementException(); }
    int res = data[head];
    data[head] = null;
    head++;
    if (head == tail) {
      head = tail = 0;
    }
    return res;
  }
}

And a test suit for it:

import org.junit.Test;
import static org.junit.Assert.*;
import java.util.*;

public class QueueTest {

  @Test
  public void testQueueN() {
    Queue q = new Queue();
    int N = 65;
    for(int i = 0; i < N; i++) {
      q.enqueue(i);
    }
    assertFalse(q.isEmpty());
    for(int i = 0; i < N; i++) {
      assertEquals(i, q.dequeue());
      assertEquals(N - i -1, q.size());
    }
    assertTrue(q.isEmpty());
  }

  @Test
  public void testEnDeEn() {
    Queue q = new Queue();
    q.enqueue(2);
    assertEquals(1, q.size());
    assertEquals(2, q.dequeue());
    q.enqueue(5);
    assertEquals(5, q.dequeue());
  }

  @Test(expected=java.util.NoSuchElementException.class)
  public void testDeOnEmpty() {
    Queue q = new Queue();
    q.dequeue();
  }
  @Test(expected=NullPointerException.class)
  public void testEnWithNull() {
    Queue q = new Queue();
    q.enqueue(null);
  }
}

Please, review it from perspective of implementation and performance.

Update: I improved(thanks to @Vogel612) this code a little bit. Improved version

Imports:

import java.util.*;

NEVER do this. Look how many classes you got now. This is ineffective and needlessly clutters your IDE's "do what I want button"-suggestions.
The fun thing is, in your main class you do it the other way round:

java.util.Arrays.sort(data, new java.util.Comparator<Object>() {

why not:
import java.util.Arrays;
import java.util.Comparator;
import java.util.NoSuchElementException;

//Later:
Arrays.sort(data, new Comparator<Object>()) {

This is shorter, does exactly the same and allows your ClassLoader to preload the classes needed --> possible performance improvements
Queue<E> and Scoping
While we're in the java.util package.. You can find an interface there.. Queue<E>. This is something you can program against. To prevent a name-conflict I'd rename your implementation to MyQueue or something similar.
You class would then begin with the following:
public class MyQueue implements Queue<Integer> {
    private Integer[] data;
    private int head;
    private int tail;

Here I also made some changes to your code.
Why? In short: You should restrict the visibility of members as much as possible. With your code, classes in the same package as your Queue could do:
Queue queue = new Queue();
queue.data[0] = Integer.MAX_VALUE;

This is not good. noone should be messing with the state of your class. That's why these members should be private (or maximally protected), but definitely not package-private which is the default scope.
same goes for your head and tail.


Printing lines of "*" of various lengths As a beginner, I tried a programming drill.  The method gets an Enumerable that contains numbers specifying how many "*" to print on each line.

public static void PrintStars(IEnumerable<int> neverEndingListOfInt)
{
    foreach (int item in neverEndingListOfInt)
    {
        Console.WriteLine(SetStars(item));
    }
}
private static string SetStars(int numberOfTimes)
{
    return string.Concat(Enumerable.Repeat("*", numberOfTimes));

}

I have another version here:

public static void PrintStars(IEnumerable<int> neverEndingListOfInt)
{

    foreach (int item in neverEndingListOfInt)
    {
        for (int i = 0; i < item; i++)
        {
            Console.Write("*");
        }
        Console.WriteLine();

    }
}

What impact on the GC will creating so many strings have?
Which way is more efficient, and how would you prefer to do it?

I don't consider myself an expert when it comes to the internals of .NET's garbage collector, but as far as I understand:

Write("*") benefits from string interning, so there should be little to no GC overhead (other than what Write does internally, of course). However, Write has to be called for every single character, and that call overhead makes it (significantly) slower.
The WriteLine variant does allocate strings (among other things), so the GC will have to run more often. Whether that has any significant impact on performance is something you'll have to measure, but it's normally not something you need to worry about.
new string('*', numberOfTimes) is more efficient (both in terms of speed and in terms of allocations) than string.Concat(Enumerable.Repeat("*", numberOfTimes)).
You may run into different kinds of issues if you allocate a lot of very large objects (larger than 85.000 bytes), due to how the Large Object Heap works. But that's probably not relevant in this case.

Note that new .NET releases occasionally contain GC improvements, so while the general principles are probably fairly static, implementation details will change over time.


Counting occurrences of each word in a sentence I have been doing this test task for C# Developer role at UBS.London recently and have been rejected. No reason stated yet. Was wondering if you can take a look and provide feedback on what I can do to improve solution. The task description is below and solution itself is on GitHub. Also attached code for two main classes I have implemented. Unit tests are in solution. No UI provided as I thought it is not required as Unit tests are self explanatory.

Hope it will be helpful for someone else doing interviews for C# Dev roles.

Exercise description

  Create a .net application that will solve the following problem. There
  are no time constraints and you are free to use any resources at your
  disposal.

  The Problem As an author I want to know the number of times each word
  appears in a sentence So that I can make sure I'm not repeating myself

  Acceptance Criteria Given a sentence When the program is run Then I'm
  returned a distinct list of words in the sentence and the number of
  times they have occurred

  Example Input: "This is a statement, and so is this." Output: this - 2
  is – 2 a – 1 statement – 1 and – 1 so - 1

Solution is on GitHub

I am also using Netfx-Guard library from Nuget

Solutions contains two projects: main project with SentenceParser and StringSplitter classes and unit testing projects.

SentenceParser and StringSplitter classes are below.

using System.Collections.Generic;
using System.Linq;
using UBS.TextParsing.Interfaces;

namespace UBS.TextParsing
{
    public class SentenceParser
    {
        private IStringSplitter _splitter;

        public SentenceParser(IStringSplitter splitter)
        {
            _splitter = splitter;
        }

        public IDictionary<string, int> Parse(string sentence)
        {
            Guard.NotNullOrEmpty(() => sentence, sentence);

            var words = _splitter.SplitIntoWords(sentence);

            var wordsCounts = from word in words
                group word by word.ToLower()
                into g
                select new {Word = g.Key, Count = g.Count()};

            return wordsCounts.ToDictionary(wc => wc.Word, wc => wc.Count);
        }
    }
}

Here is StringSplitter class which splits sentence into words.

using System;
using System.Collections.Generic;
using System.Text;
using UBS.TextParsing.Interfaces;

namespace UBS.TextParsing
{
    public class StringSplitter : IStringSplitter
    {
        public IEnumerable<string> SplitIntoWords(string sentence)
        {
            Guard.NotNull(() => sentence, sentence);

            var stringList = new List<string>();

            var currentWordSb = new StringBuilder();

            foreach (var chr in sentence)
            {
                if (Char.IsPunctuation(chr) || Char.IsSeparator(chr) || Char.IsWhiteSpace(chr))
                {
                    AddToList(currentWordSb, stringList);
                    currentWordSb.Clear();
                }
                else
                {
                    currentWordSb.Append(chr);
                }
            }

            AddToList(currentWordSb, stringList);

            return stringList;
        }

        private void AddToList(StringBuilder stringBuilder, ICollection<string> collection)
        {
            var word = stringBuilder.ToString();
            if(!string.IsNullOrEmpty(word))
                collection.Add(word);
        }
    }
}

Guard.NotNullOrEmpty(() => sentence, sentence);

This introduces a dependency on a third-party library, which is 100 lines. Why not just write

if (string.IsNullOrEmpty(sentence))
{
    throw new ...
}

The ISplitter interface is overkill. As is the StringSplitter class, probably -- as @NickUdell mentioned, a regular expression looks like the right tool for the job here.

You have the core of the solution right here:

      var words = _splitter.SplitIntoWords(sentence);

       var wordsCounts = from word in words
           group word by word.ToLower()
           into g
           select new {Word = g.Key, Count = g.Count()};

       return wordsCounts.ToDictionary(wc => wc.Word, wc => wc.Count);

Which can be simplified to

var words = _splitter.SplitIntoWords(sentence.ToLower());
return words.GroupBy(word => word)
            .ToDictionary(group => group.Key, group => group.Count());

Besides being a little shorter, we are creating fewer objects. Let's say there are \$n\$ words returned by SplitIntoWords, \$d\$ of which are distinct (up to case). The first version creates \$2n\$ string objects (the \$n\$ returned by SplitIntoWords and these same \$n\$ words lower-cased) and \$d\$ instances of the anonymous type. The second version creates \$n + 1\$ string objects. This might not be a concern, but since it's an easy win, we should just take it.

SplitIntoWords could be written using yield. Again, not a big difference but it makes the code a bit more re-usable, since client code can use Take, First, etc without having to have generate every word in the string.

public IEnumerable<string> SplitIntoWords(string sentence)
{
    var word = new StringBuilder();
    foreach (var chr in sentence)
    {
        if (Char.IsPunctuation(chr) || Char.IsSeparator(chr) || Char.IsWhiteSpace(chr))
        {
            if (word.Length > 0)
            {
                yield return word.ToString();
                word.Clear();
            }
        }
        else
        {
            word.Append(chr);
        }
    }

    if (word.Length > 0)
    {
        yield return word.ToString();
    }
}


Optimize function for assigning existing 3D array values to values of keys in array of objects I have JSON data from a Google sheet in a 3D array like this:

const values = [
  [
    // useless first array
  ],
  [
    'timestamp_dont_want',
    'bird',
    '',
    '',
    'easy',
    'foot'
  ],
  [
    'timestamp_dont_want',
    'bee',
    'celery'
  ],
  // lots of other arrays
  [
    'timestamp_dont_want',
    'bat',
    'chocolate',
    'droid',
    '',
    'finger',
    '',
    'hella cool',
    'indiana'
  ]
];

The maximum length of any given array is 12. Other than the entire first array and index 0 of every following array, each value (that is present and not empty) in an array corresponds to a desired object key label I have stored as 11 strings in another array:

const labels = [
  'label_b',
  'label_c',
  'label_d',
  'label_e',
  'label_f',
  'label_g',
  'label_h',
  'label_i',
  'label_j',
  'label_k',
  'label_l'
];

Some arrays have all the values I need and many have empty values or stop at a certain index. I would like to create a new array of objects, containing only the useful and present data, that would be organized like this:

const organized = [
  {
    label_b: 'bird',
    label_e: 'easy',
    label_f: 'foot'
  },
  {
    label_b: 'bee',
    label_e: 'celery'
  },
  // lots of other objects
  {
    label_b: 'bat',
    label_c: 'chocolate',
    label_d: 'droid',
    label_f: 'finger',
    label_h: 'hella cool',
    label_i: 'indigo'
  }
];

I wrote a function, but it doesn't feel very elegant:

function sort (arr3d, arrKeys) { // Passing values and labels constants from above
  let organized = []; // Initiate array
  for (var i = 1; i < arr3d.length; i++) { // For each array of values, starts at 1 to ignore first useless array
    organized.push({}); // Create empty objects
  }
  for (var j = 0; j < arrKeys.length; j++) { // Outer loop once for each unique object key
    for (var k = 1; k < arr3d.length; k++) { // Inner loop for each array of values, starts at 1 to ignore first useless array
      if (arr3d[k][j + 1]) { // Testing presence of value before writing in next step, ignoring index 0 of each array
        organized[k - 1][arrKeys[j]] = arr3d[k][j + 1].trim(); // Craete new key in object; trim value from associated array and assign to key
      }
    }
  }
  console.log(organized); // Prove it worked
}

Any ideas about how to refactor or optimize? Maybe you can teach me about using a new JS method.

There's two things that might help performance, and could also make your code easier to understand:

only extract the length of the arrays once per loop, instead of per iteration
deference deeply nested structures earlier, and as infrequently as possible

For #1, I normally use:

for (var i = 0, n = array.length; i < n; ++i) { ... }

For #2, look at your innermost nested line:

organized[k - 1][arrKeys[j]] = arr3d[k][j + 1].trim();

Consider reversing the order in which you enumerate over the data.  There's more scope for optimising values from the inner loop to the outer loop if you iterate over the "leftmost" components of your 2D structures first:

for (var k = 1, kn = arr3d.length; k < kn; k++) {
    var o = organized[k - 1] = {};
    var a = arr3d[k];
    for (var j = 0, jn = arrKeys.length; j < jn; j++) {
        var t = a[j + 1];
        if (t) {
            o[arrKeys[j]] = t.trim();
        }
    }
}

Note also that now that the loops are the other way around there's no need to pre-create the objects that make up the organized array - it's initialised with an empty object in the first line of the outer loop.