Separate logic in files node I have a Node project and am trying to make my code better, so I've created a new folder with a file like following which encapsulates the read and write file.

This is working and my question is if it's good practice since I need to use the call to read and write file in several place in my code.

var fs = require('fs');

function writeFile(filePath, fileContent) {
    fs.writeFile(filePath, fileContent, 'utf8', function (err) {
        if (err) {
            return console.log("error to write to file:  " + filePath + " " + err);
        }
    });
}
function readFile(filePath, content) {
    fs.readFile(filePath, 'utf8', function (err, data) {
        if (err) {
            return console.log("error to read file:  " + filePath + " " + err);
        }
        content(data);
    });
}
exports.readFile = readFile;
exports.writeFile = writeFile;

I call it in my modules like this:

module.exports = function (app) {
    app.get('/aa', function(req, res) {

        var filePath  = 'C://test.txt';
        var file = require('../helprs/file');
         file.readFile(filePath ,function(fileContent){
            console.log(fileContent);
            res.send(fileContent);
        });

    });

There are problems with your readFile and writeFile functions.  These functions are asynchronous.  As such, the callbacks you pass to fs.writeFile() and to fs.readFile() are called some time in the future, long after fs.readFile() and fs.writeFile() have already returned and finished executing.

As such, the return value you have in those callbacks just returns back into the bowels of the file I/O sub-system and does not go back to any of your code.

So, as you have the code written, you have these problems:

There's no way for the code that calls readFile() or writefile() to know when either function has completed.
There's no way for the code that calls writeFile() to know when that function succeeded or failed.
There's no way for the code that calls readFile() to know if an error occured.

Your functions will need to accept either a completion callback or they will need to return a promise because that's how the caller gets completion and error information.

Then, once you've fixed the above issues, you'll find that you've hardly changed fs.readFile() and fs.writeFile() much at all.  Pretty much all you will have done is default the encoding to utf8 since everythning will be pretty much the same.  I'd have to say that this module doesn't really seem worth it.

You could use the native fs.readFile() like this:

    var fs = require("fs");

    app.get('/aa', function(req, res) {

        var filePath  = 'C://test.txt';
        fs.readFile(filePath, 'utf8', function(err, data) {
            if (err) {
                res.send(err);
            } else {
                console.log(data);
                res.send(data);
            }
        });

    });

So, this is pretty much the same amount of code as what you had, the main difference being that this version has full error handling whereas your version did not handle errors from fs.readFile() and the browser request was left permanently with no response upon error.

Creating modules to contain shared code is a generally good idea.  But, when working with async functions and their responses, you need to make sure you preserve full async response and error handling when you write cover functions that use async library functions.  In this case, you missed some of that, rendering your cover function not fully functional.

In addition, when covering a built-in function, you should pretty much make sure you're adding enough value to justify a new interface.  Simple defaulting one argument is probably not enough change/improvement to justify creating a whole new set of functions that someone learning or modifying your code would have to become familiar with in order to be productive in your code (which would not be an issue if you just used the standard functions).

Also, in this specific example, Express has res.sendFile() which will do all this work for you:

    app.get('/aa', function(req, res) {
        res.sendFile('C://test.txt', function(err) {
            if (err) {
                res.status(err.status).end();
            }
        });
    });


Roman numeral to decimal converter Program written in TI-Basic that works on TI-83/84/+/SE calculators. Automatically detects whether input is a number of a Roman numeral, and converts to the opposite. Please try to optimize for size, if possible.

DelVar IDelVar VDelVar XDelVar LDelVar CDelVar DDelVar MClrHome
Input Str1
expr(Str1->A
If A
Then
A->B
"?->Str2
For(I,1,iPart(A/|E3
Str2+"M->Str2
B-|E3->B
End
If B>899
Then
Str2+"CM->Str2
B-900->B
End
If B>499
Then
Str2+"D->Str2
B-500->B
End
If B>399
Then
Str2+"CD->Str2
B-400->B
End
B->A
For(I,1,iPart(A.01
Str2+"C->Str2
B-|E2->B
End
If B>89
Then
Str2+"XC->Str2
B-90->B
End
If B>49
Then
Str2+"L->Str2
B-50->B
End
If B>39
Then
Str2+"XL->Str2
B-40->B
End
B->A
For(I,1,iPart(A.1
Str2+"X->Str2
B-10->B
End
If B=9
Then
DelVar BStr2+"IX->Str2
End
If B>4
Then
Str2+"V->Str2
B-5->B
End
If B=4
Then
DelVar BStr2+"IV->Str2
End
For(I,1,B
Str2+"I->Str2
End
sub(Str2,2,length(Str2)-1
Else
"?"+Str1+"?->Str1
For(I,0,5
inString(Str1,sub("IVIXXLXCCDCM",2I+1,2
If Ans
Then
sub(Str1,1,Ans-1)+sub(Str1,Ans+2,length(Str1)-Ans-1->Str1
C+(4+10fPart(I/2))10^(iPart(I/2->C
End
End
For(I,1,length(Str1
inString("IVXLCDM",sub(Str1,I,1
If Ans
C+(5-8fPart(Ans/2))10^(iPart(.5(Ans-1->C
End
C
End
Ans

You have optimized this very well. However, using a different algorithm can reduce code size from 665 bytes to 411 bytes:

DelVar IDelVar VDelVar XDelVar LDelVar CDelVar DDelVar M"?->Str2
"IVXLCDM->Str3
ClrHome
Input Str1
expr(Str1->N
If N
Then
For(I,0,log(N
10fPart(.1N->A
While A
sum({4.1,4.1,9.2}(A={4,5,9->B
sub(Str3,2I+10fPart(B)+1,1)+Str2->Str2
abs(A-1-int(B->A
End
int(.1N->N
End
sub(Str2,1,length(Str2)-1
Else
Str2+Str1+"?->Str1
For(I,0,5
inString(Str1,sub("IVIXXLXCCDCM",2I+1,2
If Ans
Then
sub(Str1,1,Ans-1)+sub(Str1,Ans+2,length(Str1)-Ans-1->Str1
C+(4+10fPart(I/2))10^(iPart(I/2->C
End
End
For(I,1,length(Str1
inString(Str3,sub(Str1,I,1
If Ans
C+(5-8fPart(.5Ans))10^(iPart(.5Ans-.5->C
End
C
End
Ans


C++ return the sum of the 2 smallest elements in a vector Here is the description of the problem from codewars:

Create a function that returns the sum of the two lowest positive numbers given an array of minimum 4 integers. No floats or empty arrays will be passed.
For example, when an array is passed like [19,5,42,2,77], the output should be 7.
[10,343445353,3453445,3453545353453] should return 3453455.
Hint: Do not modify the original array.

And this is my solution which passed all tests.
// Returns the index of the smallest number.
int smallest(std::vector<int> numbers) {
  int smallest = 1;
  for(int i = 0; i < numbers.size(); i++)
  {
    if(numbers[i] < numbers[smallest]) {
      smallest = i;
    }
  }
  return smallest;
}

long sumTwoSmallestNumbers(std::vector<int> numbers)
{
  int firstIndex = smallest(numbers);
  uint64_t first = numbers[smallest(numbers)];
  numbers.erase(numbers.begin() + firstIndex);

  uint64_t second = numbers[smallest(numbers)];

  return first + second;
}

While it's fairly fast and simple I feel like there is a way to do it while only looping over the list once and the hint about not modifying it seems to suggest that.

Missing headers

This code needs to include <vector> (for std::vector<int>) and <cstdint> (for std::uint64_t).

Don't pass collections by value

The function signature int smallest(std::vector<int> numbers) makes a copy of the input vector.  If we follow the hint that we shouldn't modify it, we can pass by reference:

int smallest(const std::vector<int>& numbers);

Even better would be to act like the standard algorithms, and not depend on the type of container at all:

template <typename Iter>
typename std::iterator_traits<Iter>::value_type
smallest(Iter first, Iter last);

Use the correct type for indexing

When indexing an collection, int isn't necessarily adequate.  Instead you should be using std::vector<int>::size_type (which is the same as std::size_t on many implementations):

using index_t = std::vector<int>::size_type;
for (index_t i = 0;  i < numbers.size();  ++i)

Use the standard algorithms

#include <algorithm>

That gives you std::partial_sort_copy().  You can fill a two-element array with the lowest two elements:

using std::begin;
using std::end;
int first_two[2];

std::partial_sort_copy(begin(nums), end(nums),
                       begin(first_two), end(first_two));

return std::accumulate(begin(first_two), end(first_two), 0);

(Yes, std::accumulate() is overkill, but it does show how to generalize this).


Error handling in controller (MVC structure) Lately, I'm using domain model pattern, and my controllers methods often looks like (yii2 example, verify user in loyalty system with Ajax response).

if (LoyaltyDomModel::loyaltyVerification($post_data)) {

    if (LoyaltyDomModel::setVerify(Yii::$app->user->identity, $post_data)) {

        return  ['body'=>'successfull','status' => true];
    }
    else {
        return  ['body'=>'unable update user','status' => 'error'];
    }
}
else {
    return  ['body'=>'errorcardno','status' => 'error'];
}

If logic is more complex I have a lot of if statement with model methods which return bool values, for example (I'm trying to decouple models logic in atomic methods), and it looks like not healthy. Should I encapsulate logic of return errors in models? What is best practices?

I advice you forego if checks completely and go for try-catch blocks, and have your verifiers throw exceptions on error. So, you do something like this:

try {
     LoyaltyDomModel::loyaltyVerification($post_data);
     LoyaltyDomModel::setVerify(Yii::$app->user->identity, $post_data);

     return  ['body'=>'successfull','status' => true];
} catch (LoyaltyVerificationException $e) {
     return ['body' => $e->getMessage(), 'status' => 'error']
} catch (UserUpdateException $e) {
     return ['body' => $e->getMessage(), 'fields' => $e->getFields(), /* you can pass custom info to exceptions, right? */, 'status' => 'error'];
}

Also, it's not a good idea to return bool in one case ('status' = true) and string in another ('status' = 'error');


Add data to DataTable I created a class to operate with concrete table and I want to generalize this code to develop more universal class.

Here is one method of this class:

public void FillColumns(DataTable table, OracleDataReader reader)
{
  if (reader != null)
    if (reader.HasRows)
    {
      table.Rows.Clear();
      while (reader.Read()) table.Rows.Add
          (
            reader["ColumnName1"],
            reader["ColumnName2"],
            reader["ColumnName3"],
            reader["ColumnName4"]
          );
    }
}

How can I refactor this concrete method, to make concrete column names as input parameters (even independing of number of columns)?

params (version 1)
You can use the same trick the Rows.Add method uses namely the params keyword

By using the params keyword, you can specify a method parameter that takes a variable number of arguments.

and pass columns names as the last parameters. Then you read the values form the reader with some LINQ and turn it into an array because it's just that, an array (but one that is created dynamically if each value is passed separately). Besides you don't need two ifs, use the and && operator instead.
public void FillColumns(
    DataTable table,
    OracleDataReader  reader,
    params string[] columnNames)
{
    if (reader != null && reader.HasRows)
    {
        table.Rows.Clear();
        while (reader.Read())
        {
            table.Rows.Add(columnNames.Select(name => reader[name]).ToArray());
        }
    }
}

Example call:
.FillColumns(table, reader, "Column1", "Column2");

table.Columns (version 2)
the second approach would be to read the column names from the data table
public void FillColumns(DataTable table, OracleDataReader reader)
{
    if (reader != null && reader.HasRows)
    {
        table.Rows.Clear();
        while (reader.Read())
        {
            table.Rows.Add(
                table
                .Columns
                .Cast<DataColumn>()
                .Select(column => reader[column.ColumnName])
                .ToArray());
        }
    }
}

table.Load (version 3)
and the third to just load the datatable from the reader:
public void FillColumns(DataTable table, OracleDataReader reader)
{
    if (reader != null && reader.HasRows)
    {
        table.Rows.Clear();
        table.Load(reader);
    }
}


Overusing JavaScript closures? I've finally gotten around to learning Lisp/functional programming. However, what I've noticed is that I'm trying to bring ideas back into JavaScript.

Example

Before

var myPlacemark,
    myLineString;

myLineString = ge.createLineString('');
myLineString.setLatitude(100);
myLineString.setLongitude(-100);
myPlacemark = ge.createPlacemark('');
myPlacemark.setGeometry(placemark);

After

var myPlacemark;

myPlacemark = (function(point, placemark){
  point.setLatitude(100);
  point.setLongitude(-100);
  placemark.setGeometry(point);
  return placemark;
})(ge.createPoint(''), ge.createPlacemark(''));

Is there any reason I shoudn't be doing it the 2nd way?

What you have there is actually just a fancy assignment operation. The closure there plays no role. And if you would need to set another placemark, you would have to repeat the code or wrap in one more function.

IMHO, it would be much more pragmatic to use a lot simpler approach:

var createPlacemark = function (point, placemark) {
        point.setLatitude(100);
        point.setLongitude(-100);
        placemark.setGeometry(point);
        return placemark;
    },
    myPlacemark = createPlacemark(ge.createPoint(''), ge.createPlacemark(''));

This way you get reusable routine with a clear name. And if goal of this all was to prevent external sources from adding placemarks, just warp it all in the standard:

(function () {

}());

The bottom line is: you were over-thinking it.


1D data classification This R code separates the values in rData. They are classified according to the class in rClass.

To do that, ClassifiedrData is a named list, where each element in the list (a class), should be a vector with all the elements in that class.

It is slow, and I had read that I should not use for in R, but I have no clue on how to vectorize it.

(I do not necessarily need a dictionary, so I welcome another data structure to store the classified numbers if it is more practical.)

I made random data (rData and rClass) as an example, but my real data is not random (so I simply creating the data classified is not a solution)

CreateEmptyDictionary <- function(names) {
    mylist.names <- names
    mylist <- vector("list", length(mylist.names))
    names(mylist) <- mylist.names
    return(mylist)
}

#Random integers
rData <- sample(x = as.integer(c(1:100)), size = 100)

#Random class
rClass <- sample(x = c(1:10), size = 100, replace = TRUE)

#Separate rData according to his class in rClass
i <- 0
ClassifiedrData <- CreateEmptyDictionary(names <- sort(unique(rClass)))

for (a in rData) {
    i <- i + 1
    #ClassifiedrData[[rClass[i]]] <-  append(ClassifiedrData[[rClass[i]]], a)
    ClassifiedrData[[rClass[i]]][length(ClassifiedrData[[rClass[i]]])+1] <- a
}

This is actually a 1-liner with the built-in split function:

ClassifiedrDataSplit <- split(rData, rClass)

You can confirm this returns the same result as your original code:

identical(ClassifiedrData, ClassifiedrDataSplit)
# [1] TRUE

In addition to being a lot less code, this will result in sensible speedups for larger datasets. For instance, consider the performance with 10 million elements and 10 classes:

rData <- sample(x = as.integer(c(1:1e7)), size = 1e7)
rClass <- sample(x = c(1:10), size = 1e7, replace = TRUE)
system.time({i <- 0
ClassifiedrData <- CreateEmptyDictionary(names <- sort(unique(rClass)))

for (a in rData) {
    i <- i + 1
    #ClassifiedrData[[rClass[i]]] <-  append(ClassifiedrData[[rClass[i]]], a)
    ClassifiedrData[[rClass[i]]][length(ClassifiedrData[[rClass[i]]])+1] <- a
}})
#    user  system elapsed
#   5.248   0.435   5.749
system.time(split(rData, rClass))
#    user  system elapsed
#   0.258   0.083   0.357

The solution with split is about 10x faster.


Populating a structure I'm solving K&R exercise 1-13:

  Write a program to print a histogram of the lengths of words
  in its input. It is easy to draw the histogram with the bars horizontal; a
  vertical orientation is more challenging.

Although the exercise itself is pretty simple, I've decided to apply a more "professional" approach and encapsulate (is that a correct word?) histogram into a structure:

typedef struct {
    /* Legend and character to 'draw' histogram with */
    char **legend, chr;
    /* Histogram data */
    int *data;
    /* Max. data value, bin width, space between bins, max. bin length */
    int datamax, binwidth, binmargin, binlength;
    /* Data and legend array lengths */
    size_t datasize, legendsize;
} chart_t;

Because it's still a toy program, I decided to not overcomplicate algorithm so my histogram is limited to only integer data (it's hard to represent floating point values graphically in console anyway).

Overall algorithm works more or less correctly, so I ask to review only my chart creation procedure. The whole point of this was to learn how to use dynamic memory (allocate, free, etc.).

#define BINWIDTH_DEFAULT 3
#define BINMARGIN_DEFAULT 1
#define BINLENGTH_DEFAULT 14

/* ... */

chart_t *create_chart(int *data, size_t ds, char **lgnd, size_t ls, char chr) {
    chart_t *out = (chart_t *) malloc(sizeof(chart_t));

    if(out == NULL)
        return NULL;

    out->chr = chr;
    out->datasize = ds;
    out->legendsize = ls;

    out->binwidth = BINWIDTH_DEFAULT;
    out->binmargin = BINMARGIN_DEFAULT;
    out->binlength = BINLENGTH_DEFAULT;

    out->data = (int *) calloc(ds, sizeof(int));

    if(out->data == NULL) {
        free(out);
        return NULL;
    }

    memcpy(out->data, data, ds);

    int dmax = 0;
    for(int i = 0; i < ds; i++)
        if(data[i] > dmax)
            dmax = data[i];

    out->datamax = dmax;

    char **legend = (char **) calloc(ls, sizeof(char *));

    for(int i = 0; i < ls; i++) {
        size_t len = strlen(lgnd[i]);

        if(len == 0)
            continue;

        char *curstr = (char *) calloc(len+1, sizeof(char));

        if(curstr == NULL) {
            if(i > 1)
                for(int j = 0; j < i; j++)
                    free(legend[j]);

            free(legend);
            free(out);

            return NULL;
        }

        strncpy(curstr, lgnd[i], len+1);

        legend[i] = curstr;
    }

    return out;
}

Haven't I missed anything? Do I have any potential memory problems in my procedure?

A case cursor == NULL does not free(out->data).
A test i > 1 is strange, and buggy. If i happens to be 1, you are leaking memory legend[0] points to.
Do not cast the result of malloc
Calling calloc in char *curstr = (char *) calloc(len+1, sizeof(char)); is an overkill:

sizeof(char) is guaranteed to be 1, and
the contents of curstr is immediately overwritten by strncpy, so its zero initialization was a waste of time.

The body of the loop spells out

    legend[i] = strdup(lgnd[i]);

in a very obfuscated way.
Given no context, it is very hard to see why do you need all that dynamic allocation at all.


is there a more functionally idiomatic way of generating valid dates in f#? open System

type Date = System.DateTime

type SafeDate =
    | WorkingDate of Date
    | InvalidDate of Exception

let dateAttempt (x: int, y:int, z:int) =
    try
        let returnDate = Date(z,y,x)
        WorkingDate returnDate
    with
        ex -> InvalidDate ex

let rnd = System.Random(4)

let randomBirthYear() =
    rnd.Next(1904,1954)
let randomBirthMonth() =
    rnd.Next(1,13)
let randomBirthDay() =
    rnd.Next(1,32)

let generateDate() =
    (randomBirthDay(), randomBirthMonth(), randomBirthYear())
    |> dateAttempt

let chooseValidDate d =
    match d with
    | WorkingDate d -> Some d
    | _ -> None

let sampleDobs = [for i in 1 .. 1000 -> generateDate()]

let cleanDates = Seq.choose chooseValidDate sampleDobs

One way to work around invalid dates is not to generate them in the first place: find the number of days between the boundary dates, generate a random integer from this range and then add that number of days to the start date:

let generateDate (min : Date) max =
    let totalDays = int (max - min).TotalDays
    let generatedDays = rnd.Next(0, totalDays)
    let generatedDate = min.AddDays(float generatedDays)
    generatedDate

let generateDateByYear minYear maxYear =
    generateDate (Date(minYear, 1, 1)) (Date(maxYear, 1, 1))

let generateBirthDate() = generateDateByYear 1904 1954

let sampleDobs = [for i in 1 .. 1000 -> generateBirthDate()]


Changing cuts to sed for parsing dates from apache log (common log file) Would switching my cuts below to SED improve performance? I am trying to get a per date count of requests for the last two weeks from a server log. The script runs, but slowly (comes in around 14 minutes). The file is 8,867,820 lines long and around 1.9G. I would guess grep, SED (or AWK) would do this more efficiently but my initial attempts failed and I resorted to cuts.

Is my piping and redirection causing unnecessary delay? Is this simply an issue of a large file?

# Log is in common log format
# host ident authuser date request status bytes
# 127.0.0.1 user-identifier frank [10/Oct/2000:13:55:36 -0700] "GET /apache_pb.gif HTTP/1.0" 200 2326

#initialize check date and end date
ckdate=$(date --date='1 days ago' +%s)
#enddate=$(date --date='1 fortnight ago' +%s)
enddate=$(date --date='3 days ago' +%s) #test date for shorter term

#feed log in reverse into loop
# tac to print log backwards into loop (newest log entry at bottom of file)
# chkdate is seconds most recent date from line is
tac "/etc/httpd/logs/access_log" | \
 while read line && (( $ckdate >= $enddate ))
do
  #send line into output, cutting out the date-time (-f4),
  # and cleaning for date only (-d: f1)
  #echo $line | cut -d ' ' -f4 | tr -d '[' | cut -d: -f1

  echo $line | cut -d ' ' -f4 | tr -d '[' \
    | cut -d: -f1 | tr '/' '-' | xargs -i date -d '{}' +'%Y-%m-%d'

  #update the check date based on latest line, formatting as seconds since 1970
  ckdate=$( ( echo $line | cut -d ' ' -f4 | tr -d '[' \
    | cut -d: -f1 | tr '/' '-' | xargs -i date -d '{}' +'%s' ) )
done | sort | uniq -c | head -n -1 | head -n 2
#put output into sorted list, with uniq counts, and trim outer bounds

Your suspicions are correct.  The amount of work that the operating system does for you to spawn a process is formidable — much more work than the string processing work itself.

For every line in the log file, you are spawning 13 processes:

cut
tr
cut
tr
xargs
date
bash — due to the use of $( ) substitution
cut
tr
cut
tr
xargs
date

Therefore, the problem can be simplified to that of extracting, reformatting, and comparing dates repeatedly using the fewest subprocesses possible.  I would even advise against any use of the date command within the loop.  The whole thing can be done with just string processing, using sed and awk.

# Log is in common log format
# host ident authuser date request status bytes
# 127.0.0.1 user-identifier frank [10/Oct/2000:13:55:36 -0700] "GET /apache_pb.gif HTTP/1.0" 200 2326

first_date=$(date --date='1 fortnight ago' +%Y-%m-%d)
last_date=$(date --date='1 day ago' +%Y-%m-%d)

# Processing the log file in reverse chronological order,
# extract the date as YYYY-MM-DD, if it is between
# $first_date and $last_date, inclusive.
#
# Then, present the hit counts in chronological order.
tac /etc/httpd/logs/access_log | \
    sed -e 's!.*\[\(../.../....\):.*!\1!' | \
    gawk -F/ -v first_date="$first_date" -v last_date="$last_date" '
        BEGIN {
            M["Jan"] = "01"; M["Feb"] = "02"; M["Mar"] = "03"; M["Apr"] = "04";
            M["May"] = "05"; M["Jun"] = "06"; M["Jul"] = "07"; M["Aug"] = "08";
            M["Sep"] = "09"; M["Oct"] = "10"; M["Nov"] = "11"; M["Dec"] = "12";
        }
        {
            DATE=sprintf("%s-%s-%s", $3, M[$2], $1);
            if (DATE < first_date) { exit }
            if (DATE <= last_date) { print DATE }
        }
    ' | \
    uniq -c | \
    tac

Some additional remarks:

I am puzzled by the fact that $enddate was defined with a granularity of one second.  I would have expected that you would want to use midnight as the limit, not whatever time of day you happen to run the script at.
I am also puzzled by the use of head -n -1 and head -n 2 to discard some of the results.  Why not just set the first and last date correctly in the first place?
Since the output from the loop is already in reverse chronological order, sort merely reverses everything.  tac would accomplish the same thing more easily.

I would also suggest that you not bother processing the log file in reverse.  Reading a file line-by-line backwards is trickier, as reading is not done sequentially, byte by byte.  Furthermore, the need to reverse the results requires buffering all of the output, preventing the script from printing results incrementally a day at a time.  Reverse chronological processing only makes sense if the log file is enormous and covers a period much much longer than the fortnight of interest.  However, if your log file is really that large, you really ought to consider a log-rolling mechanism.  If each log file only spans one day, the entire problem becomes a trivial line count exercise!


Search module in JavaScript I implemented search module, but I'm a novice in programming and I'm not sure about public and private functions that I coded. Did I do it right?

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

function My_module(string) {
    var data_size_ = data.length;

    /**
     * @param {Array.<Object>} data.
     * @return {Array.<Object>} result.
     */

    var reformat_ = function() {
        var result = new Array(data_size_);

        var first_name;
        var last_name;
        var domain;

        for (var i = 0; i < data_size_; ++i) {
            first_name = data[i].first_name.toLowerCase();
            last_name = data[i].last_name.toLowerCase();
            domain = data[i].domain.toLowerCase();

            result[i] = {
                'first_name': first_name,
                'last_name': last_name,
                'domain': domain,
            };
        }

        return result;
    };

    var formatted_data_ = reformat_();

    /**
     * A search function.
     * @return {Array.<Object>} Result
     */

    this.search = function(string) {
        var result = [];
        var len = string.length;

        for (var i = 0; i < data_size_; ++i) {
            if (formatted_data_[i].first_name.slice(0, len) === string ||
                formatted_data_[i].last_name.slice(0, len) === string ||
                formatted_data_[i].domain.slice(0, len) === string) {
                result.push(formatted_data_[i]);
            }
        }

        if (result) {
            return result;
        }
    }

}

module.exports = My_module;

Rather than creating a function named My_module, and assigning it to module.exports, you can just directly assign a function to module.exports, like this:

module.exports = function() {
    ...
}

Secondly, why are you ending all your functions and variables in My_module with an underscore, _? It doesn't do anything, and if it's not serving a real naming purpose, it can be removed.


Determining expiration date based on list of months in which to refresh I have a list of months in which users are required to update some data. I want to determine at what date they should have updated their data. They must update in March, May and August. If today is March, I want to get back the 1st of March. If today is February, I want to get back last year's August. In reality the list wouldn't be hardcoded.

I do have some working code, but I wonder if there is a simpler / more elegant of writing this (the GetCheckDate method).

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

internal class Program
{
    private static void Main()
    {
        var listOfMonths = new List<int?> {3, 5, 8};
        Console.WriteLine(GetCheckDate(listOfMonths,
            new DateTime(2014, 2, 1))); // expect 2013-08-01
        Console.WriteLine(GetCheckDate(listOfMonths,
            new DateTime(2014, 3, 1))); // expect 2014-03-01
        Console.WriteLine(GetCheckDate(listOfMonths,
            new DateTime(2014, 7, 1))); // expect 2014-05-01
        Console.WriteLine(GetCheckDate(null,
            new DateTime(2014, 7, 1))); // expect 2013-07-1
        Console.WriteLine(GetCheckDate(new List<int?>(),
            new DateTime(2014, 7, 1))); // expect 2013-07-1
    }

    private static DateTime GetCheckDate(IEnumerable<int?> listOfMonths,
        DateTime curDate)
    {
        DateTime result = curDate.AddYears(-1);
        if (listOfMonths == null) return result;
        listOfMonths = listOfMonths.ToList();
        if (!listOfMonths.Any()) return result;

        listOfMonths = listOfMonths.OrderByDescending(m => m).ToList();
        var greatestSmallerThan =
            listOfMonths.FirstOrDefault(m => m <= curDate.Month)
            ?? listOfMonths.FirstOrDefault(m => m > curDate.Month);
        result = new DateTime(
            greatestSmallerThan.Value <= curDate.Month
                ? curDate.Year : curDate.Year - 1, greatestSmallerThan.Value, 1);

        return result;
    }
}

I suggest you to improve naming:

months instead of listOfMonths because its not list, and its better not to include variable type description into name
date instead of curDate - its better to avoid abbreviations, also current method should not care whether moth is current or not - it just search previous month from given months
Possibly I would rename method to something like GetPreviousMonth

Also I don't see any need to have nullable type for months sequence. It should have integers. If month exist in list, then you can avoid sorting - so do this check first. Thus you are not going to add/remove month, then saving ordered sequence to array is little better:

public static DateTime GetCheckDate(IEnumerable<int> months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.AddDays(1 - date.Day);

    var orderedMonths = months.OrderByDescending(m => m).ToArray();
    if (date.Month > orderedMonths.First())
        return new DateTime(date.Year, orderedMonths.First(), 1);

    if (date.Month < orderedMonths.Last())
        return new DateTime(date.Year - 1, orderedMonths.First(), 1);

    return new DateTime(date.Year, orderedMonths.First(m => m < date.Month), 1);
}

But I also prefer readability on first place (performance should be improved only if its a problem) so I would go with Max and Min month instead of sorting them and keeping in mind what is Last and what is First:

public static DateTime GetCheckDate(IEnumerable<int> months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.AddDays(1 - date.Day);

    if (months.Max() < date.Month)
        return new DateTime(date.Year, months.Max(), 1);

    if (date.Month < months.Min())
        return new DateTime(date.Year - 1, months.Max(), 1);

    return new DateTime(date.Year, months.Where(m => m < date.Month).Max(), 1);
}

If I would continue making it more readable, then following extension methods would be useful:

public static DateTime FirstDayOfMonth(this DateTime date, int month = 0)
{
    return new DateTime(date.Year, month == 0 ? date.Month : month, 1);
}

public static DateTime PreviousYear(this DateTime date)
{
    return date.AddYears(-1);
}

Now whole method is very easy to understand:

public static DateTime GetCheckDate(IEnumerable<int> months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.FirstDayOfMonth();

    if (months.Max() < date.Month)
        return date.FirstDayOfMonth(months.Max());

    if (date.Month < months.Min())
        return date.PreviousYear().FirstDayOfMonth(months.Max());

    return date.FirstDayOfMonth(months.Where(m => m < date.Month).Max());
}


Very simple interpreter I created a very simple interpreter in Python. It's capable of doing basic math (unfortunately with only two numbers), creating quines and printing text.

import re

def prompt():
    userInput = input("> ")
    return userInput

def parse(command):
    if command.startswith("+"):
        numbers = re.findall(r'\d+', command)
        nums = ''.join(numbers)
        nums.replace(" ", "")
        firstNumber = int(nums[0])
        secondNumber = int(nums[1])
        print(firstNumber + secondNumber)

    elif command.startswith("-"):
        numbers = re.findall(r'\d+', command)
        nums = ''.join(numbers)
        nums.replace(" ", "")
        firstNumber = int(nums[0])
        secondNumber = int(nums[1])
        print(firstNumber - secondNumber)

    elif command.startswith("*"):
        numbers = re.findall(r'\d+', command)
        nums = ''.join(numbers)
        nums.replace(" ", "")
        firstNumber = int(nums[0])
        secondNumber = int(nums[1])
        print(firstNumber * secondNumber)

    elif command.startswith("/"):
        numbers = re.findall(r'\d+', command)
        nums = ''.join(numbers)
        nums.replace(" ", "")
        firstNumber = int(nums[0])
        secondNumber = int(nums[1])
        print(firstNumber / secondNumber)

    elif command.startswith("q"):
        print(command)

    elif command.startswith("p"):
        print(command.strip("p "))

while True:
    userInput = prompt()
    parse(userInput)

Some examples:

Input: +65

Output: 11

Input: q This is a quine

Output: q This is a quine

It's working good for me, but there's a few things I'd like to add, like multiple statements on one line and math with 2+ numbers.

Is there anything I could do to improve this?

Please remember I'm not a professional programmer and that this is one of the first interpreters I've ever written.

First off, great that you have a separate prompt function. Too often people don't do this and it's much better separated into it's own function. However, you keep it very light on purpose right now. Much better would be to perform input validation here, ensure that the user has provided something you can actually work with. Even at a basic level, you could test for an empty string:

def prompt():
    while True:
        userInput = input("> ")
        if len(userInput) > 0:
            return userInput
        print("Please enter a command.")

You can see I used a while loop to keep asking for input until the user enters something valid, and then the function returns whatever their command is. This is only a small amount of validation, but it should give you an idea about how to expand on the idea.

You should have more small functions like prompt. For example, your number extracting process is the same in each arithmetic function but you copy and paste it anyway. Instead, have one function to handle that in all cases.

def parse_numbers(user_input):
    numbers = re.findall(r'\d+', user_input)
    nums = ''.join(numbers)
    return nums.replace(" ", "")

One thing that's odd about this function is that it only allows for single digit numbers. You should use the actual operator to split up separate numbers and that way you can have multiple digits.

>>> "12+251".split('+')
["12", "251"]

With this, I'd modify parse_numbers to instead split(+):

def parse_numbers(user_input, operator):
    numbers = user_input.split(operator)

Note that we nowhave to pass the operator parameter, this way we can call parse numbers on any arithmetic command, whether it has a +, -, * or \.

We could simply return numbers now, but then we'd have no input validation. So instead lets try turning all the string into numbers before we return them. A good way to do this is using map, which will apply a function to every item in a list. So we can apply int to each number and then return it. We can also use try except to make sure that if anything can't be converted to a number we tell the user what happened.

def parse_numbers(user_input, operator):
    numbers = user_input.split(operator)
    try:
        return map(int, numbers)
    except ValueError as error:
        print (error.message)

except ValueError means that if an invalid value is passed to int (like "hello") Python will catch the error. This prevents the whole program from ending and gives you an opportunity to instead inform the user why their input is invalid. That's what print (error.message) does, as that will tell the user something useful like:

invalid literal for int() with base 10: 'hello'

To continue with all these functions, I think you should have a function for each command. They can be quite short now that you defined parse_numbers but they still help organisation:

def add(command):
    numbers = parse_numbers(command)
    return numbers[0] + numbers[1]

Easy, right? And you could do this with all of them. Note that I'm using return instead of print because it's best to have functions actually return values in case you ever need them for more than just displaying the result. Also, it's not much harder to account for more than two numbers, ie. "23+3+1". It's very easy for add, you can use a built in function called sum that gets the total sum of all elements in a list.

def add(command):
    numbers = parse_numbers(command, '+')
    return sum(numbers)

For other functions it's not so easy, but you can use something called reduce. reduce basically takes each element of the list and combines them through some function until all the elements have been combined. This does exactly what you need for the other cases. For reduce you need to pass a list as well as a function, in this case we can use a special function called a lambda, they're basically small one line functions. They're useful in cases like this where you don't need to bother defining a whole function to do the job. They're quite compact but still look like functions just without names. Here's how subtract would look:

def subtract(command):
    numbers = parse_numbers(command, '-')
    return reduce(lambda x, y: x - y, numbers)

The same principle can operate on * and / to give you a set of four arithmetic functions. In Python 3 this function was moved to the functools module, so you'll need add an import for it.

from functools import reduce

Of course your other two are even simpler, they're just basic print calls.

def quine(command):
    print(command)

def print_command(command):
    print(command.strip("p "))

Two points on these though. First, I have no idea what a quine is. I had to google it and I still don't see why it's implemented here? You could explain it with a comment, but now that it's a function you could put on a docstring to explain what it does. Docstrings are programmatically accessible comments that explain a function or class. Good practice to put them on your functions. Even if add is quite clear in its use and function, it's good to note that it operates on a list of any length of integers and returns a single integer.

Second, your print command will accidentally remove any 'p's from the end of a string. strip actually removes all the characters that it's passed, not just a specific string. So see what happens here:

>>> "p ppp p p p ppppp pp p ".strip("p ")
""

Instead, just using slicing. Slicing allows you to use indices to get a certain range from a string or list. In your case, you just want to get from the 3rd element on. Indexing starts at 0, so the third element is index 2. You can slice command with [2:]. The lack of a second parameter after : means that it will go to the end of the string.

Now, since we have a bunch of handy functions, this will make the parse function even neater. Here's how it could look:

def prompt():
    while True:
        userInput = input("> ")
        if len(userInput) > 0:
            return userInput
        print("Please enter a command.")

def parse_numbers(user_input, operator):
    numbers = user_input.split(operator)
    try:
        return map(int, numbers)
    except ValueError as error:
        print (error.message)

def add(command):
    numbers = parse_numbers(command, '+')
    return sum(numbers)

def subtract(command):
    numbers = parse_numbers(command, '-')
    return reduce(lambda x, y: x - y, numbers)

def multiply(command):
    numbers = parse_numbers(command, '*')
    return reduce(lambda x, y: x * y, numbers)

def divide(command):
    numbers = parse_numbers(command, '/')
    return reduce(lambda x, y: x / y, numbers)

def parse(command):
    if command.startswith("q"):
        quine(command)
    elif command.startswith("p "):
        print_command(command)
    elif '+' in command:
        print (add(command))
    elif '-' in command:
        print (subtract(command))
    elif '*' in command:
        print (multiply(command))
    elif '/' in command:
        print (divide(command))
    else:
        print("Invalid command")

You'll note I added a print to inform the user when they've passed a totally invalid command too. I'm also not keen on the long chain of elifs, you could remove them partially with dictionaries since they're all basically calling a function with the same parameter. The difficulty is figuring out how to extract the command from your string without clashes. This is more of a design than programming problem, because if you're going to extend this you need to find a safe way to define your syntax that's both computer and human readable.


Code review for style, clarity and efficiency. Removing duplicates from linkedlist import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;

class RemoveDuplicates {
    public static void main(String[] args) {
        LinkedList<Integer> list = new LinkedList<Integer>();
        list.add(1);
        list.add(1);
        list.add(1);
        list.add(4);
        System.out.println(RemoveDuplicates.rmDuplicates(list));
    }

    public static List<Integer> rmDuplicates(List<Integer> list) {
        Collections.sort(list);
        Iterator<Integer> it = list.iterator();
        int index = 0;
        while (it.hasNext() && index < list.size() - 1) {
            int current = it.next();
            index++;
            if (current == list.get(index)) {
                it.remove();
                index--;
            }
        }
        return list;
    }
}

Update: I did not want a sorted list. Just removing duplicates is enough and I wanted it in place. I thought without sorting it would be tricky to remove duplicates.

LinkedList is designed to preserve the order in which elements were inserted into it above all other considerations.  It should only be used when you need to retrieve the items in the list in order, because there is a faster collection available for any other type of access.

It looks from your code like you want a sorted list of elements, in addition to duplicates removed as stated in your question.  Sets are designed to have no duplicates and TreeSet is designed to sort your items as they are inserted.  If you use a TreeSet instead of a LinkedList you don't need a rmDuplicates method at all!  If you are concerned about returning a Set instead of a List, you can always return a Collection because it has about the same signature.  Time the difference between your method and using a TreeSet with a hundred thousand elements.  I think you will be surprised how fast TreeSet is!

public static void main(String[] args) {
    TreeSet<Integer> list = new TreeSet<Integer>();
    list.add(1);
    list.add(1);
    list.add(1);
    list.add(4);
    System.out.println(list);
}

If you needed to preserve the insertion order, I'd use a LinkedHashSet:

public static Collection<Integer> rmDuplicates(Collection<Integer> list) {
    return new LinkedHashSet(list);
}

If you have tried this with a custom type of Object instead of an integer and it didn't work, then you need to implement equals(), hashcode(), and maybe compareTo() properly.


Refactor java methods that do not have a common hierarchy but have same API I have this piece of code where the class ExecutionMessage has three variants. The point is that the classes are autogenerated by Apache CXF and they do not have a common class. But all the three flavors have the same API. Is there a way to refactor the same using generics or Java 8 semantics to avoid the repetition of code.

public void convertToList (ExecutionMessageV2 mesg, List<ErrorDetail> errors) {
    if (mesg == null) {
        return;
    }
    ErrorDetail err = new ErrorDetail(mesg.getCode(),mesg.getDescription(), mesg.getSourceExceptionName());
    if (mesg.getSeverity() != null)
        err.setSeverity(mesg.getSeverity().value());
    if (mesg.getType() != null)
        err.setType(mesg.getType().value());
    errors.add(err);
    convertToList(mesg.getDetail(), errors);
}

public void convertToList (ExecutionMessageV1_0 mesg, List<ErrorDetail> errors) {
    if (mesg == null) {
        return;
    }
    ErrorDetail err = new ErrorDetail(mesg.getCode(),mesg.getDescription(), mesg.getSourceExceptionName());
    if (mesg.getSeverity() != null)
        err.setSeverity(mesg.getSeverity().value());
    if (mesg.getType() != null)
        err.setType(mesg.getType().value());
    errors.add(err);
    convertToList(mesg.getDetail(), errors);
}

public void convertToList (ExecutionMessageV2_0 mesg, List<ErrorDetail> errors) {
    if (mesg == null) {
        return;
    }
    ErrorDetail err = new ErrorDetail(mesg.getCode(),mesg.getDescription(), mesg.getSourceExceptionName());
    if (mesg.getSeverity() != null)
        err.setSeverity(mesg.getSeverity().value());
    if (mesg.getType() != null)
        err.setType(mesg.getType().value());
    errors.add(err);
    convertToList(mesg.getDetail(), errors);
}

Are you able to change your code to rely on a wrapper class then?  - myself

Going on a limb here, but do you want to consider using a wrapper class to encapsulate the fact that you have three similar generated classes, and provide a common way of accessing/modifying them? This assumes:

The methods on the generated classes have the same parameter and return types.
For the case of getDetail() method, that returns the same type as the input object, i.e. recursion.

If both conditions satisfy, then a sample wrapper implementation can be as such (using poorly named class names and variables, please excuse that):

public class WrapperClass {

    private final C1 o1;
    private final C2 o2;

    private WrapperClass(C1 o1, C2 o2) {
        this.o1 = o1;
        this.o2 = o2;
    }

    public String getCode() {
        return findAny(mapC1(C1::getCode), mapC2(C2::getCode));
    }

    // other getter methods

    private <T> T mapC1(Function<C1, T> mapper) {
        return Optional.ofNullable(o1).map(mapper).orElse(null);
    }

    private <T> T mapC2(Function<C2, T> mapper) {
        return Optional.ofNullable(o2).map(mapper).orElse(null);
    }

    private static <T> T findAny(Object... items) {
        return (T) Arrays.stream(items).filter(Objects::nonNull).findAny().orElse(null);
    }

    public static WrapperClass withC1(C1 o1) {
        return new WrapperClass(o1, null);
    }

    public static WrapperClass withC2(C2 o2) {
        return new WrapperClass(null, o2);
    }
}

You will need a map...(Function<?, T>) method for each oh your message class that performs the conversion, and the findAny(Object...) method to get the desired non-null value, if applicable. Each getter method, e.g. getCode() as shown, is relatively trivial to implement. In your case, since you are only dealing with three different classes (which is already a lot), you can also consider a simpler if-ladder instead of streaming through the values.

A slight benefit of the wrapper class is that it lets you perform additional conversions too, for example to an ErrorDetails object:

public final WrapperClass {

    // ...

    public ErrorDetails toErrorDetails() {
        ErrorDetails result = new ErrorDetails(getCode(), getDescription(),
                                                getSourceExceptionName());
        Optional.ofNullable(getSeverity())
                .map(Severity::value)
                .ifPresent(result::setSeverity);
        Optional.ofNullable(getType())
                .map(Type::value)
                .ifPresent(result::setType);
        return result;
    }

    // ...

}

Your current implementations of (convertToList()) will then just become:

public void convertToList(C1 o1, List<ErrorDetails> errors) {
    convertToList(WrapperClass.withC1(o1), errors);
}

public void convertToList(C2 o2, List<ErrorDetails> errors) {
    convertToList(WrapperClass.withC2(o2), errors);
}

private void convertToList(WrapperClass wrapper, List<ErrorDetails> errors) {
    errors.add(wrapper.toErrorDetails());
    convertToList(wrapper.getDetail(), errors);
}

Implementing the wrapper class's getDetail() will be quite similar, other than the slight difference that it should also return its own type:

public WrapperClass getDetail() {
    return new WrapperClass(mapC1(C1::getDetail), mapC2(C2::getDetail));
}


Word Jumble in Python This game that I semi coded is a little bulky and kind of boring. I want to know if there is anything simple I can do to shrink it and add a little spice to the code.

# Word Jumble
#
# The computer picks a random word and then "jumbles" it
# The player has to guess the original word

import random

# create a sequence of words to choose from
WORDS = ("python", "jumble", "easy", "difficult", "answer", "xylophone")

print(
"""
           Welcome to Word Jumble!

   Unscramble the letters to make a word.
(Press the enter key at the prompt to quit.)
"""
)

play=input("Do you want to play? (yes or no)")
while play=="yes":
    # pick one word randomly from the sequence
    word = random.choice(WORDS)
    # create a variable to use later to see if the guess is correct
    correct = word

    # create a jumbled version of the word
    jumble =""
    while word:
        position = random.randrange(len(word))
        jumble += word[position]
        word = word[:position] + word[(position + 1):]

    print("The jumble is:", jumble)
    points=100
    guess = input("\nYour guess: ")
    while guess != correct and guess != "":
        print("Sorry, that's not it.")
        hint=input("Do you need a hint?")
        if hint=="yes":
            points=int(points)-10
            if correct=="python":
                print("Its a snake...")
            elif correct=="jumble":
                print("Rhymes with rumble")
            elif correct== "easy":
                print("This one is so simple!")
            elif correct=="difficult":
                print("This is a hard one... its very ________________")
            elif correct=="answer":
                print("You cant find it? the _________ is ___________")
            elif correct=="xylophone":
                print("It is a toy...")
            print("Thanks for takeing the hint, idiot...")
        guess = input("Your guess: ")

    if guess == correct:
        print("That's it!  You guessed it!\n")
        print("Your score is: "+str(points))
        play=input("Do you want to play again? (yes or no)")
    elif guess== "":
        print("You failed...")
        play=input("Do you want to play again? (yes or no)")

print("Thanks for playing.")

input("\n\nPress the enter key to exit.")

Some comments from this question still apply here.

You do not respect PEP 8 and you should try to split your code into smaller chunks.

Let's change things little by little :

Data over code

Copied from my other answer

Sometimes, you have to write a lot of code because "Hey, I have a lot of logic to write, I have to write code, that's the point of programming" but the point is more to keep things simple and to use the right tool (which is not always code) for the right thing.

Here you are comparing strings to get the relevant hint for an answer. It would be much clearer to store the hint are the words together in a structure. At the moment, I've decided to store this in a list of tuple (a dictionnary would also have done the trick).

WORDS = (
    ("python", "Its a snake..."),
    ("jumble", "Rhymes with rumble"),
    ("easy", "This one is so simple!"),
    ("difficult", "This is a hard one... its very ________________"),
    ("answer", "You cant find it? the _________ is ___________"),
    ("xylophone", "It is a toy..."),
)
...
    # pick one word randomly from the sequence
    word, word_hint = random.choice(WORDS)
...
        print(word_hint)
        print("Thanks for takeing the hint, idiot...")

Extracting logic in a function

The logic to create a jumble version looks like it could and should be put in a function on its own.

Also we can reuse already existing functions : shuffle.

Now, the whole code is much simpler (also, you don't need word AND correct) :

def get_jumble_word(word):
    l = list(word)
    random.shuffle(l)
    return ''.join(l)

...
    word, word_hint = random.choice(WORDS)
    print("The jumble is:", get_jumble_word(word))

A bif of logic

You have :

while guess != word and guess != "":
    # code with no break
if guess == word:
    foo()
elif guess == "":
    bar()

After the loop, we know that condition guess != word and guess != "" is not true anymore (because we would have stayed in the loop otherwise). In order for this condition not to be true, we must have : guess == word or guess == "".

Thus, if condition guess == word in the if branch is not true, we always go in the guess == "" part.

You can rewrite this :

if guess == word:
    foo()
else:
    assert guess == ""
    bar()

Do not repeat yourself

Do not repeat yourself.

Do not repeat yourself.

You have the same last line in both branches of :

if guess == word:
    print("That's it!  You guessed it!\n")
    print("Your score is: " + str(points))
    play = input("Do you want to play again? (yes or no)")
else:
    print("You failed...")
    play = input("Do you want to play again? (yes or no)")

It probably should be :

if guess == word:
    print("That's it!  You guessed it!\n")
    print("Your score is: " + str(points))
else:
    print("You failed...")
play = input("Do you want to play again? (yes or no)")

More functions

You can define a function to handle getting the yes value from the user, one to handle a game, one to handle the interface (asking yes/no between games) :

def get_input_in_list(prompt, values):
    while True:
        s = input(prompt + '(Your choices are : ' + ', '.join(values) + ')')
        if s in values:
            return s

def game():
    # pick one word randomly from the sequence
    word, word_hint = random.choice(WORDS)
    print("The jumble is:", get_jumble_word(word))
    points = 100
    guess = input("\nYour guess: ")
    while guess != word and guess != "":
        print("Sorry, that's not it.")
        hint = input("Do you need a hint?")
        if hint == "yes":
            points = int(points) - 10
            print(word_hint)
            print("Thanks for takeing the hint, idiot...")
        guess = input("Your guess: ")

    if guess == word:
        print("That's it!  You guessed it!\n")
        print("Your score is: " + str(points))
    else:
        print("You failed...")

def main():
    while input("Do you want to play? (yes or no)") == "yes":
        game()

if __name__ == "__main__":
    main()

Also, I took this chance to add an if main guard.


Checking whether any anagram of a string appears within another string I have the following code that takes a string t and checks to see if any anagram of t is a substring of s:

### Question 1 main function and helper functions.
"""Given two strings s and t, determine whether some anagram of t is a
substring of s. For example: if s = "udacity" and t = "ad", then the
function returns True. Your function definition should look like:
question1(s, t) and return a boolean True or False."""
# Define an anagram.
def anagram(s1, s2):

    # anagram is true if sorted values of s1 and s2 are equal.
    s1 = list(s1)
    s2 = list(s2)
    s1.sort()
    s2.sort()
    return s1 == s2

# Main function.
def Question1(t, s):

    # Defining local variables.
    t_len = len(t)
    s_len = len(s)
    t_sort = sorted(t)

    # Setting the range to be that of t.
    for i in range(s_len - t_len + 1):

        # Call helper function which checks for t in s.
        if anagram(s[i: i+t_len], t):
            return True
    return False

# Simple test case.
print Question1("app", "paple")
# True

# Test case with a space character.
print Question1("", "a space")
# True

# Test case with numbers.
print Question1("15", "4732890793878894351")
# True

# Test case with punctuation.
print Question1("$ %", "100% $Expensive")
# True

# Test case that returns false.
print Question1("music", "muscle")
# False

# End question 1.
print """---End Question 1---
"""

Is this code Industry standard, and if not what can I do that would simplify this code and increase it's efficiency?

The asymptotic time complexity of your implementation is:
$$O(\ (s-t)*(t*log(t))\ )$$

Because you sort and compare \$s-t\$ substrings of length \$t\$.
(I'm assuming Python uses \$n*log(n)\$ sort).

Here is a solution with \$\  O(\ t*(s-t+log(t))\ ) \ \$ time complexity.

It uses a sorted sliding window of length \$t\$. When the window slides through s, an old character is removed and a new one is added into a specific position, such that the window is kept sorted.

Initial sorting of string t is \$\ t*log(t)\$
Initial sorting of the sliding window is \$\ t*log(t)\$
Initial comparison of sorted(t) with the window is \$\ t\$
For each character in s[len(t):], total of \$\ s-t\ \$ items, there is:

one bisection of window for the character to be removed : \$\ log(t)\$
one removal of a character : \$\ t\$
one bisection of window for the character to be inserted : \$\ log(t)\$
one insertion of a character : \$\ t\$
one comparison of sorted(t) with the window : \$\ t\$

So the asymptotic complexity is \$O(\ t*log(t)\ +\ (s-t)*t\ )\$,
or simply \$O(\ t*(s-t+log(t))\ )\$.

from bisect import bisect_left, bisect_right

def question1_fast(t, s):

  if len(s) < len(t):
    return False

  t = sorted(t)
  window = sorted(s[:len(t)])

  if window == t:
    return True

  for i, c_new in enumerate(s[len(t):]):

    c_old = s[i]

    if c_old < c_new:
      idx_old = bisect_right(window, c_old) - 1
      idx_new = bisect_left(window, c_new) - 1

    elif c_new < c_old:
      idx_new = bisect_right(window, c_new)
      idx_old = bisect_left(window, c_old)

    else:
      continue

    del window[idx_old]
    window.insert(idx_new, c_new)

    if window == t:
      return True

  return False

A little comparison of efficiency on large s and t:

>>> from random import randint, shuffle
>>>
>>> # `s` is a string of 10,000 random characters
>>> s = ''.join(chr(randint(32,127)) for _ in range(10000))
>>>
>>> # `t` is a random permutation of `s[5000:6000]`
>>> t_list = list(s[5000:6000])
>>> shuffle(t_list)
>>> t = ''.join(t_list)
>>>
>>> timeit('Question1_original(t,s)', globals=locals(), number=10)
25.469552749997092
>>> timeit('Question1_Caridorc(t,s)', globals=locals(), number=10)
12.726046560999748
>>> timeit('question1_fast(t,s)', globals=locals(), number=10)
0.10736406500291196

With small strings, the differences are not so significant:

>>> timeit('Question1_original("ad", "udacity")', globals=locals())
4.730070723002427
>>> timeit('Question1_Caridorc("ad", "udacity")', globals=locals())
6.0496803390014975
>>> timeit('question1_fast("ad", "udacity")', globals=locals())
3.89388234800208

EDIT

Scratch that. I just realized there's a more efficient way to do it. One that is \$\ O(s+t)\ \$ in time complexity.

My original idea was to have a window which is a sorted list, and keeping the window sorted the whole time, all for the sake of the comparison window == t. If window and t weren't both sorted, the comparison obviously wouldn't work as needed.

But to find out whether one string is an anagram of another string, we don't have to sort them, we just need to know if they contain the same characters. Sorting and comparing them is one way. A more efficient way is to count the number of occurrences of each distinct character. The result would be a dictionary mapping to each character the number of its occurrences. Then you just compare the dictionary of str1 with the dictionary of str2, and if they are equal, the strings are anagrams of each other.

The comparison of dictionaries is not much more efficient than comparing sorted lists. If anything, I suspect it might be even slower. It's the insertions and deletions, which are nearly \$O(1)\$ in dictionary, but \$O(t)\$ in the case of a sorted list.

Since neither t nor window have to be sorted, neither be kept sorted, the time complexity is drastically reduced:

Creation of dictionary for t : \$\ t\$
Creation of dictionary for window : \$\ t\$
For each character in s[len(t):] (repeated \$s-t\$ times):

one decrementation of counter : \$\ 1\$
one incrementation of counter : \$\ 1\$

That adds up to \$\ O(\ t+(s-t)\ )\ \$ or just \$\ O(s+t)\$.

def make_counter_dict(s, t):
  counter_dict = dict.fromkeys(s, 0)
  for c in t:
    counter_dict[c] += 1
  return counter_dict

def question1_even_faster(t, s):

  if len(s) < len(t):
    return False

  t_dict = make_counter_dict(s, t)
  window = make_counter_dict(s, s[:len(t)])

  for c_old, c_new in zip(s, s[len(t):]):

    if window == t_dict:
      return True

    window[c_new] += 1
    window[c_old] -= 1

  return window == t_dict

Performance (measured with another random s and t of the same parameters as before):

>>> timeit('question1_fast(t,s)', globals=locals(), number=1000)
10.559728353000537
>>> timeit('question1_even_faster(t,s)', globals=locals(), number=1000)
2.1034470079976018

This version is slower on small strings though:

>>> timeit('question1_fast("ad", "udacity")', globals=locals())
3.886769873002777
>>> timeit('question1_even_faster("ad", "udacity")', globals=locals())
4.691746060998412


Understanding the differences: A direct comparison between Cython and C++ Introduction:

The following two code samples are a direct comparison of performance between Cython and C++. I pieced them together with the help of others-- I barely understand how they work. Their speed in my computer is: 1.225s vs 3.989s (C++ being the faster). The purpose of this comparison is to decide which language I will be using for a series of small but computationally intensive scripts (~200-500 lines).

Question:

As a novice Python programmer dealing with data types, memory (de)allocation, and other low level processes for the very first time, what should I keep in mind when writing Cython modules? Is there anything in the Cython code that can be improved?

Cython script:

import cython
import numpy as np

@cython.boundscheck(False)
@cython.wraparound(False)

cdef class MathObject:
    cdef double a

    def __init__(self, a):
        self.a = a

    # chaotic function (simple math that can't be simplified out by the compiler).
    cpdef double logMap(self):
        self.a = 4.0 * self.a * (1.0 - self.a)
        return 0

    def get_value(self):
        return self.a

def run():
    cdef int i, ii, iii
    cdef int one_thousand = 1000
    cdef double x = 0.2  # logMap has complex behavior at this value

    cdef MathObject objInstance

    objArray = np.empty(one_thousand, dtype=MathObject)
    for i in range(len(objArray)):
        objArray[i] = MathObject(x)

    cdef MathObject[:] objArrayiew = objArray

    for i in range(one_thousand):
        for ii in range(one_thousand):
            for iii in range(one_thousand):
                objInstance = objArrayiew[iii]
                objInstance.logMap()
        print objArray[i].get_value()

    return None

C++ script:

#include <iostream>
using namespace std;

class MathObject {
    public:

    double a;
    MathObject(double b) {
        a=b;
    };

    // simple chaotic function
    double logMap()
    {
        a = 4.0*a*(1.0-a);
    } // logMap
};

int main(int argc, char *argv[])
{
    int ii,jj,kk;

    // create an array of 1000 objects
    int N = 1000;
    MathObject *objArray [N];  // 1000 pointers to objects
    for (ii=0; ii<N; ++ii)  {
        objArray[ii] = new MathObject(0.2);  // construct new objects
    } // for ii

    for (kk=0; kk<1000; ++kk)  {
    for (jj=0; jj<1000; ++jj)  {
    for (ii=0; ii<N; ++ii)  {
        objArray[ii] -> logMap();
    } // for ii
    } // for jj
    } // for kk

    for (ii=0; ii<N; ++ii)  {
        cout << objArray[ii] -> a << "\n";
    } // for ii
}

(Note: I know that this question is a little vague, but most existing tutorials for Cython are written for an audience of C++ programmers. Beginner level information is astoundingly helpful!)

(Second Note: A good explanation of part of the Cython script can be found in the answer to this question.)

Translating python to C++ (just a direct line for line translation).

Headers

import cython                                       #include <iostream>
import numpy as np                                  #include <vector>
                                                    #include "Range.h"
@cython.boundscheck(False)
@cython.wraparound(False)

MathObject

cdef class MathObject:                              struct MathObject {
    cdef double a                                       double a;

    def __init__(self, a):                              MathObject(double a)
        self.a = a                                          : a(a)
                                                        {}
    cpdef double logMap(self):                          double logMap() {
        self.a = 4.0 * self.a * (1.0 - self.a)              a = 4.0 * a * (1.0 - a);
        return 0                                            return 0;
                                                        }
    def get_value(self):                                double get_value() {
        return self.a                                       return a;
                                                        }
                                                    };

run

def run():                                          void run() {
    cdef int i, ii, iii
    cdef int one_thousand = 1000                        int one_thousand = 1000;
    cdef double x = 0.2                                 double x = 0.2;

    cdef MathObject objInstance

    objArray = np.empty(one_thousand, dtype=MathObject) std::vector<MathObject> objArray(one_thousand, x);
    for i in range(len(objArray)):
        objArray[i] = MathObject(x)

    cdef MathObject[:] objArrayiew = objArray

    for i in range(one_thousand):                       for(int i: Range(one_thousand)) {
        for ii in range(one_thousand):                      for(int ii: Range(one_thousand)) {
            for iii in range(one_thousand):                     for(int iii: Range(one_thousand)) {
                objInstance = objArrayiew[iii]                      MathObject& objInstance = objArray[iii];
                objInstance.logMap()                                objInstance.logMap();
                                                                }
                                                            }
        print objArray[i].get_value()                       std::cout << objArray[i].get_value() << "\n";
                                                        }

    return None
                                                    }

main

if __name__ == "__main__":                          int main() {
    run()                                               run();
                                                    }

In C++ there is no built in Range type. But it is easy to write:

class Range
{
    int beginValue;
    int endValue;

    struct RangeIterator
    {
        int value;
        RangeIterator(int v) : value(v) {}
        RangeIterator& operator++()     {++value;return *this;}
        int            operator*()      {return value;}
        bool           operator!=(RangeIterator const& rhs) const {return value != rhs.value;}
    };

    public:
        typedef RangeIterator           iterator;
        Range(int e)
            : Range(0, e)
        {}
        Range(int b, int e)
            : beginValue(b), endValue(e)
        {}

        iterator begin() { return RangeIterator(beginValue);}
        iterator end()   { return RangeIterator(endValue);}
};

A couple of notes on the direct translation that I would change:

If you are not going to return a meaningful value (and we notice from usage that the return value is ignored).

double logMap() {
    a = 4.0 * a * (1.0 - a);
    return 0;
}

Then I would make this a void function so you know its not supposed to return anything.

void logMap() {
    a = 4.0 * a * (1.0 - a);
}

I don't like geters they break encapsulation:

double get_value() {
    return a;
}

You usually find that if you are getting values from an object doing stuff with it there is usally a member function (or member of the public API) that can do that. In this case you are only using the value to print. So why not just create a function that knows how to print the value.

friend std::ostream& operator<<(std::ostream& stream, MathObject const& data)
{
    return stream << data.a;
}

If values don't change then mark them as const.

int    const one_thousand = 1000;
double const x            = 0.2;


Counting binary substrings The exercise i am working on requires me to read a binary sequence stored in a text file. The objective is to find the occurrences of all subsequences of length between a and b (their values are given in the function parameters) and put them in a list of tuples.
For example: If the following binary sequence is given as input and the length of each subsequence should be between 2 and 4:
a = 2
b = 4

01010010010001000111101100001010011001111000010010011110010000000

I should write a program to generate the following output:
[(1, ["1011", "1101"]),
 (2, ["0101", "0110", "1010"]),
 (3, ["0111", "101", "1110", "1111"]),
 (4, ["0001", "0011", "1100"]),
 (5, ["011", "1000", "110"]),
 (6, ["0000", "111"]),
 (7, ["0010", "1001"]),
 (8, ["0100"]),
 (10, ["010"]),
 (11, ["000", "001", "11"]),
 (12, ["100"]),
 (15, ["01", "10"]),
 (23, ["00"] )]

You will notice that each tuple in the list has two elements. The first element is the number of times a subsequence appears in the given sequence whilst the second element is the list containing the corresponding subsequences.
E.g. the first tuple indicates that the subsequences "1011" and "1101" appear only once in the given sequence.
The solution i have written does solve the problem (it passes all correctness tests), but it is slow and doesn't pass the performance tests (it only passes 5 out of 22 tests) in which the maximum timeout is set to 1 second. How could i make this solution faster?
I am not allowed to import any external libraries.
def ex1(ftesto,a,b,n):
    with open(ftesto,encoding='utf8') as f:
        S = f.read().replace('\n','')
    N = len(S)
    D = {}
    subsequences = {}
    for i in range(N+1):
        for j in range(i+1,N+1):
            if a <= len(S[i:j]) <= b:
                if S[i:j] not in D:
                    D[S[i:j]] = 1
                else:
                    D[S[i:j]] += 1
    for k,v in D.items():
        subsequences.setdefault(v,[]).append(k)
    result = sorted([(k,sorted(v)) for k,v in subsequences.items()])
    return result[:n]

if __name__ == '__main__':

   ftesto = 'ft1.txt'
   a = 2
   b = 4
   n = 20

The amount of elements in the list must be equal or less than "n". That explains why i am using the slicing at the end of the function.

PEP 8
You are violating several Style Guide for Python Code rules:

commas should be followed by a space,
variables should be snake_case,
always surround binary operators with one space

Never called
ex1() is never called by your mainline.
Declare variables closer to where they are used
subsequences is declared 7 lines earlier than it needs to be.
Separate I/O from processing
ex1() cannot be called with test data, unless that test data is in a file.  Use two functions.  One that solves the problem with an in-memory string, and a second that reads the string from a file for processing.
Avoid dict.setdefault(...)
subsequences.setdefault(v, []).append(k) will create a new list ([]) every time it is executed.  Many times this default will never be used, and must be garbage collected.  This is just wasting time.
Instead, use collections.defaultdict.  Note that collections is not an external library; it is internal, built into Python.
Counter
Similarly, use collections.Counter for counting objects.  Again, built into Python; not an external library.
Reworked Code

from collections import Counter, defaultdict

def binary_sequence_counts(sequence, shortest, longest, limit):
    n = len(sequence)

    sequence_counter = Counter()
    for i in range(n + 1):
        for j in range(i + 1, n + 1):
            if shortest <= len(sequence[i:j]) <= longest:
                sequence_counter[sequence[i:j]] += 1

    subsequences = defaultdict(list)
    for subsequence, count in sequence_counter.items():
        subsequences[count].append(subsequence)

    result = sorted((count, sorted(subseqs)) for count, subseqs in subsequences.items())
    return result[:limit]

def ex1(ftesto, a, b, n):
    with open(ftesto, encoding='utf8') as f:
        sequence = f.read().replace('\n', '')

    return binary_sequence_counts(sequence, a, b, n)

def testcase():
    seq = "01010010010001000111101100001010011001111000010010011110010000000"
    result = binary_sequence_counts(seq, 2, 4, 20)
    # print result, or
    # check if result is correct

The above code is way more readable than the original.
Efficiency
This code is just plain awful:
    for i in range(n + 1):
        for j in range(i + 1, n + 1):
            if shortest <= len(sequence[i:j]) <= longest:
                ...

If your sequence is 100,000 digits long, the inner loop will execute about 5,000,000,000 times!  That's a lot!  Especially if you're only concerned with subsequences between 2 and 4 digits long, in which case you only need about 300,000 iterations.
A simple modification of the range(...) limits would dramatically improve your speed.  If you do it correctly, you can even get rid of the a <= len(...) <= b test, because with the correct ranges you will not be able to generate sequences that are the incorrect length.
In fact, with the correct limits, you could even use the counter to do the counting entirely for you:
sequence_counts = Counter(sequence[i:j]
                          for i in range(...)
                          for j in range(...))

Proper range limits left as exercise to student.
min heap
You are sorting a potentially huge list of values, and then returning only the n-smallest results.  sorted(iterable)[:n]  This can be a potentially huge waste of time!
Using the heapq.nsmallest(n, iterable) (again, built-in, not an external library), you can reduce the amount of space used, and possibly the amount of time.  Note that this is faster only for smaller values of n.  If n is big, it can be faster to use sorted(iterable)[:n].  Always profile!
Roll Your Own
The question has been updated to disallow any imports.  No problem.  We'll just have to implement our own defaultdict and Counter.
Assuming your are allowed to create your own class ...
object.__missing__
When a dict.__getitem__ is called, but the dictionary does not contain that key, object.__missing__(self, key) is called.  We can use this method to change the behaviour.
Counter
To implement a simple "counter" dictionary, we just need 0 to be returned for any missing key.
class Counter(dict):
    def __missing__(self, key):
        return 0

If the key is missing, 0 is returned.  If you "add 1" to a missing key, 0 is returned, 1 is added to the returned value, and the resulting 1 is stored back under that key:
>>> c = Counter()
>>> c['foo']
0
>>> c['bar'] += 1
>>> c['bar'] += 1
>>> c
{'bar': 2}

Notice that the 'foo' key still does not exist; only the 'bar' key exists because values were actually written to it.
defaultdict
You don't actually need a full defaultdict here.  You just need a dictionary which returns an empty list when an unknown key is retrieved.  Unlike the Counter class above, we need to remember the list that was returned for the key, because we'll be appending items to the list, which (unlike the += operator) won't write the result back into the dictionary.
class DictOfLists(dict):
    def __missing__(self, key):
        self[key] = []
        return self[key]

Unlike the Counter, referencing an unknown key will create the key in the dictionary, filling it with an empty list:
>>> dol = DictOfLists()
>>> dol['foo']
[]
>>> dol['bar'].append("baz")
>>> dol
{'foo': [], 'bar': ['baz']}
>>>

The actual defaultdict allows you to specify a factory function in the constructor, to create values for the missing keys.  That isn't too much more complicated.  Feel free to research this.


RSpec test for pagination This is a Rspec test for pagination in Rails project. I'm not sure if I should write the test in spec/requests or spec/controllers.

And there must be a lot of thing that I had better to do. Which part of code should I refactor?

spec/requests/companies_spec.r

describe "Companies" do
  describe "GET /companies" do
    before(:all) { 50.times { FactoryGirl.create(:company) }}
    describe "index" do
      context "with 50 companies" do
        it "has not second page" do
          visit root_path
          expect(page).to have_no_xpath("//*[@class='pagination']//a[text()='2']")
        end
      end
      context "with 51 companies" do
        before{ FactoryGirl.create(:company) }
        it "has second page" do
          visit root_path
          find("//*[@class='pagination']//a[text()='2']").click
          expect(page.status_code).to eq(200)
        end
      end
    end
  end
end

There are many ways to test this. Mostly, though, it'd be nice to avoid having to create 50+ records, since it slows down your tests.

If you use a request spec, though, it's probably best to create 50+ records, since it's a high-level test, so you'll want to be close to the "real" usage scenario.

But you can cheat a little in other places. For instance, if you have the records-per-page number defined in a way that's configurable, you can set it to something lower in you pagination test (or you can set it globally for the test environment). For instance, if the per-page is set to 2, you only need to create 3 records to test pagination. That'll be a lot faster than creating 51 records.

If you're spec'ing the view itself, you can simply define the instance variables that'll trigger pagination links, and not bother with the actual records. Or you can use FactoryGirl.build_list to merely build the records and assign them to a view-accessible variable, without actually storing them in the database - again, faster.

You can also look into mocking and stubbing to avoid actually creating the records.

For your current code, You can do a couple of things, like:

describe "Companies" do
  describe "GET /companies", order: :defined do
    before(:all) { FactoryGirl.create_list :company, PER_PAGE }

    context "with few records" do
      it "does not paginate records" do
        visit "/companies"
        expect(page).to have_no_xpath("//*[@class='pagination']//a[text()='2']")
      end
    end

    context "with many records" do
      it "paginates records" do
        FactoryGirl.create :company

        visit "/companies"
        expect(page).to have_xpath("//*[@class='pagination']//a[text()='2']")
        find("//*[@class='pagination']//a[text()='2']").click
        expect(page.status_code).to eq(200)
      end
    end
  end
end

Changes I've made:

Using FactoryGirl.create_list to create a number of records at once.
Using a PER_PAGE constant, just in case it isn't 50. This could also be an ENV var, an instance variable, or simply hard-coded. But naming it helps document the code.
Using order: :defined to force the examples to be run in the order they're defined. This avoids the specs randomly failing because the 2nd test has been run before the first one.
I've change the visit path to /companies because that's what the spec is about. You used visit root_path, which no doubt worked fine, but the spec is about visiting /companies, so I find it nicer to keep it consistent.

You might also want to check that the correct records actually show up on the page. I.e. attempt to find the name of the 51st company within the rendered page, when you've gone to the 2nd page's path.

Lastly, you may want to add some specs for how the system should behave if you go say page 4, but there aren't enough records to show anything.

But again, I'd probably start with view/controller specs, before moving on to high-level request specs. Request specs are great because they test everything pretty close to actual usage. But that also makes them more complex, so the more you can check at a lower level, the better.


Determining position of point W.R.T to given line equation Problem from HackerRank::

  Problem Statement:

  There are \$N\$ lines. Each line has an index between \$1\$ and \$N\$. The slope of each line is negative, i.e. it goes from upper-left to lower-right.

  There are \$Q\$ queries. Each of them is in the format \$L\$ \$R\$ \$x\$ \$y\$, and you should output whether there is any line with index between \$L\$ and \$R\$ and the point \$(x,y)\$ is under it. If there is, then the answer isYES, otherwiseNO`.

  As you know, any line splits an infinite plane into two regions. The point \$(x,y)\$ is under the line if that point is at the same region with point \$(-\infty , -\infty)\$ . If the point lies on the line it does not count.

  Input Format:

  The first line contains \$N\$, the number of lines. The following \$N\$ lines each contains two integers \$m\$ and \$n\$ that describes the line \$mx + n = y\$.

  Output Format:

  For each query, output one line containing either YES or NO.

  Sample Input:

2
-1 3
-2 -4
3
1 2 0 0
1 1 0 0
2 2 0 0

  Sample Output:

YES YES NO

  Explanation:

For this, I've submitted this code in Python, but in half of the test case, the time limit was exceeded.

n = int(input())
x = []
y = []
for i in range(n):
    l = list(map(int, input().split()))
    x.append(l[0])
    y.append(l[1])
q = int(input())

for i in range(q):
    p = list(map(int, input().split()))
    L = p[0]-1
    R = p[1]-1
    X = p[2]
    Y = p[3]
    flag = 0
    for i in range(L,R+1,1):
        if(Y - x[i]*X - y[i] < 0):
            print("YES")
            flag = 1
            break;
    if(flag == 0):
        print("NO")

I can't understand how can I improve my code further. Because for loop to check from L to R is necessary. also the loop to execute every query is necessary. So in total 2 loops are needed. How can I reduce time here?

Note to potential future reviewers: This code appears to be suitable for Python 3 only.

Let's structure your code a little and see if we can fix the timing problem.

I propose the following order:

Global variables
Helper functions
Main function

Input
Actual calculations

if __name__ == '__main__':

The latter may not look familiar to you, but it's an include guard. It's a good practice to start using those, especially if you ever want to re-use parts of your code. Now your main only gets executed if the script is called directly, not when it's called by import.

Which leaves us after the first couple of edits with the following:

x = []
y = []

def ask_amount():
    return int(input())

def ask_lines(amount):
    for i in range(amount):
        l = list(map(int, input().split()))
        x.append(l[0])
        y.append(l[1])

def ask_queries(amount):
    return list(map(int, input().split()))

def main():
    ask_lines(ask_amount())
    q = ask_amount()
    for i in range(q):
        p = ask_queries(q)
        L = p[0]-1
        R = p[1]-1
        X = p[2]
        Y = p[3]
        flag = 0
        for i in range(L, R+1, 1):
            if(Y - x[i]*X - y[i] < 0):
                print("YES")
                flag = 1
                break
        if(flag == 0):
            print("NO")

if __name__ == '__main__':
    main()

Which still isn't much, but the structure is getting better. Note that I made your comma's breath ((L,R+1,1) is more readable when written as (L, R+1, 1) and there's no longer a ; behind your break. It's Python, we don't need semi-colons after break/continue etc.

One of the major things fixed now is your code was executing before you finished reading your input. That's usually a bad idea unless you're in some kind of do until told otherwise loop, which this wasn't. Now it waits for all input before it executes the rest.

I/O used to be:

2
-1 3
-2 -4
3
1 2 0 0
YES
1 1 0 0
YES
2 2 0 0
NO

Now it is:

2
-1 3
-2 -4
3
1 2 0 0
1 1 0 0
2 2 0 0
YES
YES
NO

There's still a couple of problems with it. We're now iterating an extra time, which can't be helpful for the time it takes to execute. But at least it looks better now, so we can see what we're doing. It still fails just as many test cases as your original code did, so at least we didn't do noticeably worse.

What else can we do better?

    L = query[0]-1
    R = query[1]-1
    X = query[2]
    Y = query[3]
    flag = 0
    for i in range(L, R+1, 1):

R = some value minus one. The actual value we need is R plus one. Since the description of the challenge says L and R are given (not L+1 and R+1), it doesn't make sense to modify them.

Let's go from L-1 to R instead.

    L = query[0]
    R = query[1]
    X = query[2]
    Y = query[3]
    flag = 0
    for i in range(L-1, R, 1):

Another thing we don't need is this entire flag business. Let's use return values instead:

x = []
y = []

def ask_amount():
    return int(input())

def ask_lines(amount):
    for i in range(amount):
        l = list(map(int, input().split()))
        x.append(l[0])
        y.append(l[1])

def ask_queries(amount):
    return list(map(int, input().split()))

def does_line_exist(query):
    L = query[0]
    R = query[1]
    X = query[2]
    Y = query[3]
    for i in range(L-1, R, 1):
        if(Y - x[i]*X - y[i] < 0):
            return True
    return False

def main():
    ask_lines(ask_amount())
    q = ask_amount()
    queries = []
    for i in range(q):
        queries.append(ask_queries(q))
    for query in queries:
        if does_line_exist(query):
            print("YES")
        else:
            print("NO")

if __name__ == '__main__':
    main()

Poof! No more flag.

But wait, this still doesn't solve your TLE problem! No, I'm afraid not and I'm going to tell you why. The author of the problem has made the constraints so narrow that the only acceptable solutions are those which use the Convex hull trick. It basically boils down to checking whether a value is < -y instead of < 0.

I'll leave the actual implementation of said trick to you. Feel free to post a follow-up question once you've implemented it.


Filter a list of integers by parity Exercise 2.20

  The procedures +, *,
  and list take arbitrary numbers of
  arguments. One way to define such
  procedures is to use define with
  dotted-tail notation. In a procedure
  definition, a parameter list that has
  a dot before the last parameter name
  indicates that, when the procedure is
  called, the initial parameters (if
  any) will have as values the initial
  arguments, as usual, but the final
  parameter's value will be a list of
  any remaining arguments.

  For instance, given the definition

(define (f x y . z) <body>)

  the procedure f can be called with two
  or more arguments. If we evaluate

(f 1 2 3 4 5 6)

  then in the body of f, x will be 1, y
  will be 2, and z will be the list (3 4
  5 6). Given the definition

(define (g . w) <body>)

  the procedure g can be called with
  zero or more arguments. If we evaluate

(g 1 2 3 4 5 6)

  then in the body of g, w will be the
  list

(1 2 3 4 5 6)

  Use this notation to write a procedure
  same-parity that takes one or more
  integers and returns a list of all the
  arguments that have the same even-odd
  parity as the first argument. For
  example,

(same-parity 1 2 3 4 5 6 7)
(1 3 5 7)

(same-parity 2 3 4 5 6 7)
(2 4 6)

I wrote the following:

(define (same-parity n . l)
  (define (even? n) (= (remainder n 2) 0))
  (define (odd? n) (not (even? n)))
  (define (rec n . l)
    (define (include-and-go)
      (cons (car l) (apply rec (cons n (cdr l)))))
    (define (exclude-and-go)
      (apply rec (cons n (cdr l))))
    (if (null? l) null
        (if (or (and (even? (car l))
                     (even? n))
                (and (odd? (car l))
                     (odd? n)))
            (include-and-go)
            (exclude-and-go))))
  (cons n (apply rec (cons n l))))

Is there a better way?

If one were to abstract out the filter function (named filt here), it would greatly simplify writing same-parity:

(define (filt test? lis)
  (cond
    ((null? lis) lis)
    ((test? (car lis)) (cons (car lis) (filt test? (cdr lis))))
    (else (filt test? (cdr lis)))))

(define (same-parity n . lis)
  (filt (if (odd? n) odd? even?) lis))

The filter function is common enough that it is part of the Scheme standard and is appropriately named filter.


Creating a masonry effect with varying height rectangles The following is used to create a masonry effect with varying height rectangles. How can I simplify and make this more elegant in Unity?

private float startX = 0.0f;
private float startY = 0.0f;
private float posX = 0.0f;
private float posY = 0.0f;
private float prevWidth = 0;
private float prevHeight = 0;
private int index = 0;
private int currentCol = 0;

private void AddCard(int index, int col, float width, float height)
{
    GameObject goCard = (GameObject)GameObject.Instantiate(pfCard00);
    PackedSprite psCard = goCard.GetComponentInChildren<PackedSprite>();

    goCard.name = "QuestItemCard" + index.ToString();
    goCard.transform.parent = goCardContainer.transform;
    psCard.height = height;

    if (currentCol != col)
    {
        posX += width;

        posY = 0;
        prevHeight = 0;
    }

    // placement
    posY += (prevHeight / 2) + (height / 2);
    goCard.transform.localPosition = new Vector3(startX + posX, startY - posY, goCard.transform.position.z);

    if (currentCol != col)
    {
        prevWidth = width;

        currentCol = col;
    }

    prevHeight = height;
}

Usage:

AddCard(0, 0, 100, 100);
AddCard(1, 0, 100, 100);
AddCard(2, 0, 100, 100);
AddCard(3, 0, 100, 200);

AddCard(4, 1, 100, 100);
AddCard(5, 1, 100, 100);
AddCard(6, 1, 100, 100);
AddCard(7, 1, 100, 200);

Consider splitting the code that increments the column into a separate method that can be called manually. Now you do not have to pass in column or row, and index can be incremented by the AddCard method. This greatly simplifies usage and allows you to make use of a for loop.

private float startX = 0.0f;
private float startY = 0.0f;
private float posX = 0.0f;
private float posY = 0.0f;
private float prevWidth = 0;
private float prevHeight = 0;
private int index = 0;
private int currentCol = 0;

private void IncreaseColumn()
{
    currentCol++;
    posX += width;
    posY = 0;
    prevHeight = 0;
}

private void AddCard(float width, float height)
{
    GameObject goCard = (GameObject)GameObject.Instantiate(pfCard00);
    PackedSprite psCard = goCard.GetComponentInChildren<PackedSprite>();

    goCard.name = "QuestItemCard" + index.ToString();
    goCard.transform.parent = goCardContainer.transform;
    psCard.height = height;

    // placement
    posY += (prevHeight / 2) + (height / 2);
    goCard.transform.localPosition = new Vector3(startX + posX, startY - posY, goCard.transform.position.z);

    prevWidth = width;
    prevHeight = height;

    index++;
}

Usage:

var columns = 2;
var rows = 4;

for(int x= 0; x< columns; x++)
{
    for(int y = 0; y<rows-1; y++)
    {
        AddCard(100, 100);
    }
    AddCard(100, 200);
    IncreaseColumn();
}


Rectangularize a list-of-lists structure Considering I have an iterated "lists of lists ..." (up to 4 dimensions), now I wish to make the full list "rectangular" (or well each dimension equal in size). For a 4 dimensional case I've written out a specific version:

x=[[[[4,3,4,5],[1],[2,2]],[[3,6,7],[2,3],[1]]],[[[1]]]]

length0 = len(x)
length1 = len(sorted(x,key=len, reverse=True)[0])
length2 = 0
length3 = 0
for xi in x:
    lengthi = len(sorted(xi,key=len, reverse=True)[0])
    for xii in xi:
        lengthii = len(sorted(xii,key=len, reverse=True)[0])
        length3 = max(lengthii, length3)
    length2 = max(lengthi, length2)
tlist3 = [None]
tlist2 = [tlist3 * length3]
tlist1 = [tlist2 * length2]

for xi in x:
    for xii in xi:
        for xiii in xii:
            xiii.extend(tlist3*(length3 - len(xiii) ))
        xii.extend(tlist2*(length2 - len(xii)))
    xi.extend(tlist1 * (length1 - len(xi)))

print(x)

It works, but that's everything I can say about this code (it doesn't look clean, nor is it scalable to more dimensions easily).

Ideally I would've a function that goes up to N dimensions into a given list, and rectangulars everything

As a side note: this is for insertion into a NumPy array.

You can find a maximum without sorting. Instead of

len(sorted(x,key=len, reverse=True)[0])

you can use either of these:

len(max(x, key=len))
max(len(s) for s in x)

For a more generic solution I propose this:

import itertools

def rectangularize(nested_list, dimensions, fill_value=None):
    flat = nested_list
    for dim in range(dimensions-2, -1, -1):
        size = max(len(s) for s in flat)
        for s in flat:
            s.extend([] if dim else fill_value for _ in range(size - len(s)))
        flat = list(itertools.chain.from_iterable(flat))

x=[[[[4,3,4,5],[1],[2,2]],[[3,6,7],[2,3],[1]]],[[[1]]]]
rectangularize(x, 4)
print(x)

To explain:

The flat list is a flattened view of one dimension of nested_list. It contains references to sublists of nested_list; extending the sublists through flat mutates nested_list.
flat = list(itertools.chain.from_iterable(flat)) advances flat to the next dimension by concatenating the sublists.
The main loop runs dimensions-1 times because the outermost list never needs to be extended.
I'm using range(dimensions-2, -1, -1) to make dim == 0 in the final iteration where a different fill value is needed, so I can use a clean if dim else to deal with that.


Object can throw exception on construction, but I don't want it to stop everything I have a factory that loads configuration from an xml file, and uses the configuration to be able to create objects of an appropriate type with appropriate settings. My application does a number of different tasks, and I don't want a broken or missing xml file to interfere with the other tasks the application does.

The factory is a Guice-provided dependency, however, if I make the constructor throw an exception, then I will just get a ProvisionException, which could cause unpredictable results or crash the application, neither of which are acceptable.

Currently, I have the factory swallow the exception on creation, and then throw it when the make method is called. Alternatively, I have a makeUnchecked method, if you call the ready() method first. Are there any problems with this approach, or is there potentially a better way to do it?

SSCCE:

public class FooFactory {
    private Exception ex = null;

    @Inject
    public FooFactory(@FooSettings String filename) {
        loadSettings(filename);
    }

    public void loadSettings(String filename) {
        try {
            // stuff
            this.ex = null;
        } catch(Exception e) {
            this.ex = e;
        }
    }

    public Foo make(InputObj input) throws Exception {
        if(ex != null) throw ex;
        makeUnchecked(input);
    }

    public Foo makeUnchecked(InputObj input) {
        if (ex != null) throw new IllegalStateException(ex);
        // factory stuff
    }

    public boolean ready() {
        return ex == null;
    }
}

My application does a number of different tasks, and I don't want a broken or missing xml file to interfere with the other tasks the application does.

If you don't need the XML file configuration, why do you have it? How can you continue without it?

How you answer this conceptual question has a lot to do with how you implement the code logic using exceptions.