Lottery number picker This is my first Python project. Let me know how I did, and where I can improve it.

# Random Lottery Number Pick Tool
# It has U.S. national lottos and two from Nebraska
# I'll add more to it later.

import random

# here is a giant body function I wonder if I could make this easier to deal with?
# it is basically my program's menu items.

def workerFunction():
    while True:
        try: x=input( '\n\n Select an option:\n\n'
                      'Press 1+Enter for Pick Three\n'
                      'Press 2+Enter for Pick Five\n'
                      'Press 3+Enter for Powerball picks\n'
                      'Press 4+Enter for Mega Millions picks\n'
                      'Q+Enter to quit \n\n'  )

        #This is my end of file logic that kills the program
        # Provided you pass Q + Enter

        except EOFError: pass
        if x.lower().startswith('q'):
            print("done")
            break

        # Otherwise you are passing something else that requires work from my factory!

        elif x == '1': print( '\n\n Nebraska Pick Three',
                              factory.repeater( 3, factory.factoryLogic, [0, 9, 1] ) )
        elif x == '2': print( '\n\n Nebraska Pick Five',
                              factory.factoryLogic( 1, 38, 5 ) )
        elif x == '3': print( '\n\n Powerball',
                              factory.factoryLogic( 1, 55, 5 ),
                              factory.factoryLogic( 1, 42, 1 ) )
        elif x == '4': print( '\n\n Mega Millions',
                              factory.factoryLogic( 1, 75, 5 ),
                              factory.factoryLogic( 1, 15, 1 ) )

# My factory class (self is set to something)...
# Not sure how to use it yet outside of the program I know it is like "this" in javascript.

class factory:

    # so I am defined self which looks like a struct. I assign it 0 (to shut up the syntax checker)

    def __init__( self ): self.a = 0

    # My logic module picks numbers based on a start and end position.
    # Not sure why I use +1 in the range function.
    # And then there are the number of interators I need.

    def factoryLogic( startPosi, endPosi, interateNumber ):
        a = random.sample( range( startPosi, endPosi+1 ), interateNumber )
        a.sort()
        return a

    # This is a repeater utility I made because pick three needs to be called three times.
    # The neat thing is that I am als passing a functions and args as a list and breaking them out.
    # I could make it more terse but why? It also returns an appended list. Neat!

    def repeater( times, f, functionArgs ):
        return_list = []
        for i in range( times ): return_list.append( f( functionArgs[0], functionArgs[1], functionArgs[2]  ) )
        return return_list

# This name main holds my worker function. I call it workerFunction! Amazing, yes?
if __name__ == '__main__':
    workerFunction()

You are not compliant with the style guide. For example:

x and a aren't good variable names;
You don't follow the naming convention (e.g. factory should be Factory and workerFunction should be worker_function);
The inline comments should be replaced with docstrings;
Extraneous whitespace (e.g. for i in range( times ): should be for i in range(times):); and
Compound statements (e.g. except EOFError: pass should be on two lines).

The Pythonic replacement for lots of elif clauses is a dictionary. There are numerous ways to implement this, e.g.:

lotteries = {1: {'name': 'Nebraska Pick Three',
                 'func': factory.repeater
                 'args': (3, factory.factoryLogic, [0, 9, 1])},
             ...}

This simplifies the printout at the start:

print("Make a choice: ")
for key in sorted(lotteries):
    print("{0}. {1}".format(key, lotteries[key][name]))

and input validation:

if int(x) not in lotteries:

and the output:

lottery = lotteries[x]
print lottery['func'](*lottery['args'])

If you do want to stick with the current text block, Python does have multiline strings:

s = """Three quote marks
       makes a string
       multiline."""

For help on formatting, see Avoiding Python multiline string indentation.

The class implementation seems pointless - you have an __init__ that does very little (self.a is never referenced again), and two instance methods that don't need any class or instance attributes. If you want an OOP approach, I would be tempted by something like:

class Lottery:

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

    def draw(self):
        ...

You can then put instances into the dictionary:

lotteries = {1: {'name': 'Nebraska Pick Three',
                 'lottery': Lottery(...)},
             ...}

and the code gets even simpler:

lottery = lotteries[x]['lottery']
lottery.draw()

You will have to come up with a method for representing the different rules. I would suggest having a Boolean for when repeat draws are allowed, rather than a sample, instead of calling a separate method as you do now. Different lotteries could alternatively be subclasses, rather than instances, of Lottery.

Here's an example implementation (note: for Python 3.x):

import random

class Lottery:
    """Simulate a specified lottery."""

    def __init__(self, name, rules):
        """Define the lottery name and draw rules."""
        self.name = name
        self.rules = rules

    def draw(self):
        """Simulate a single draw of the lottery."""
        output = []
        for rule in self.rules:
            output.extend(self._draw_rule(rule))
        return output

    def _draw_rule(self, rule):
        """Helper function to draw a single rule."""
        if rule.get('repeats', False):
            output = [random.randint(rule.get('min', 1), rule['max'])
                      for _ in range(rule.get('picks', 1))]
        else:
            output = random.sample(range(rule.get('min', 1), rule['max']+1),
                                   rule.get('picks', 1))
        return output

lotteries = {1: Lottery('Nebraska Pick Three',
                        [{'picks': 3, 'max': 9, 'repeats': True}]),
             2: Lottery('Nebraska Pick Five',
                        [{'picks': 5, 'max': 38}]),
             3: Lottery('Powerball',
                        [{'picks': 5, 'max': 75}, {'max': 15}])}

def play():
    """Allow the user to choose a lottery and simulate a draw."""
    print("Select a lottery:")
    for key in sorted(lotteries):
        print("{0}: {1.name}".format(key, lotteries[key]))
    while True:
        try:
            choice = int(input("Make a choice: "))
        except ValueError:
            print("Please enter an integer.")
        else:
            if choice in lotteries:
                break
            print("Not a valid choice.")
    print(lotteries[choice].draw())

if __name__ == "__main__":
    play()

And its output:

>>> play()
Select a lottery:
1: Nebraska Pick Three
2: Nebraska Pick Five
3: Powerball
Make a choice: foo
Please enter an integer.
Make a choice: 42
Not a valid choice.
Make a choice: 1
[1, 7, 9]
>>> play()
Select a lottery:
1: Nebraska Pick Three
2: Nebraska Pick Five
3: Powerball
Make a choice: 2
[34, 33, 20, 7, 35]
>>> play()
Select a lottery:
1: Nebraska Pick Three
2: Nebraska Pick Five
3: Powerball
Make a choice: 3
[75, 4, 12, 31, 55, 2]

Here I have used a simple dictionary implementation of the rules, with sensible defaults to minimise the input needed.


Rendering column headers, some of them with a cellRenderer attribute How can I make this code block shorter? I'm repeating myself, because of this prop cellRenderer.

{headers.map((header, i) => {
  return header.index ? (
    <Column
      label={header.label}
      dataKey={header.id}
      disableSort
      width={100}
      flexGrow={1}
      cellRenderer={this._generateCheckbox}
    />
  ) : (
    <Column
      label={header.label}
      dataKey={header.id}
      disableSort
      width={100}
      flexGrow={1}
    />
  );
})}

Passing undefined in for cellRenderer should be the same as not passing it in at all. Because of this, the duplication can be removed by doing the following:

{headers.map((header, i) => {
  return (
    <Column
      label={header.label}
      dataKey={header.id}
      disableSort
      width={100}
      flexGrow={1}
      cellRenderer={header.index ? this._generateCheckbox : undefined}
    />
  )
})}

Alternatively, you can build up an extraProps object and then spread it into the column.

{headers.map((header, i) => {
  let extraProps = header.index ? {
    cellRenderer: this._generateCheckbox
  } : {}
  return (
    <Column
      label={header.label}
      dataKey={header.id}
      disableSort
      width={100}
      flexGrow={1}
      {...extraProps}
    />
  )
})}


Program to check an item's price against available credit This is a working program to check an item's price against available credit. I'm trying to simplify this program to the bare minimum. It would require to run so I'll be better able to understand each part of the 'if' and 'else' processes. Is there a simpler way to write a program that accomplishes the task below? I'm new to C#, just trying to figure this stuff out.

  Write a program named CheckCredit that prompts users to enter a purchase price for an item.
  If the value entered is greater than a credit limit of $8,000, display you have exceeded the credit limit; otherwise, display Approved.

using static System.Console;

namespace CheckCredit
{
    class Program
    {
        static void Main(string[] args)
        {
            const double CreditCheck = 8000;
            string userInput;
            double price;

            WriteLine("This is a program designed to check an item's price
            against your amount of available credit.");
            WriteLine("Your credit limit is $8,000.00.\n");

            do
            {
                Write("Please type the item's price:");
                userInput = ReadLine();
                if (!double.TryParse(userInput, out _))
                {
                    WriteLine("Invalid input, please enter a whole or decimal number.");
                    userInput = null;
                }
            } while (!double.TryParse(userInput, out price));

            if (price > CreditCheck)
            {
                WriteLine(" You have exceeded the credit limit", price);
            }

            else if
            (price == CreditCheck)
            {
                WriteLine(
                "Approved.(*)\n\n\n"
                +
                "(*) It is exactly your credit limit.");
            }
            else
            {
                WriteLine("Approved.");
            }
            ReadKey();
        }
    }
}

Since you are a beginner, I will try to go easy on you.  That said, this looks very beginner-ish.

As noted in the comments above, you inadvertently put a line break which causes a compiler error.  There are many here you would CLOSE the question on that alone.

Beginner's frequently use double for binary floating point values.  However, anytime you are working with money or currency, then you should use Decimal.  Decimal is floating point as well but it's Base 10 rather than Base 2.

The static import of System.Console may be allowed but in general is frowned upon.  My eyes would rather see Console.WriteLine instead of WriteLine due to over dozen years of writing .NET apps.

Naming is important.  CreditCheck is a bad name.  It should be creditLimit or currentBalance if not simply balance.  But "Check" is an action verb and it would be a method that would perform that checking action, not a variable.  Note too that for local variable naming I am using camelCase.

Organization is less than desired.  Beginners will frequently put everything in Main method.  How about you create a separate method and pass the credit limit as an argument to it?  Or even a method that accepts the credit limit and item price?

You get to decide whether such a method should return a bool to denote you have sufficient funds, or perhaps return the remaining funds after the price.  If it returned remaining funds, you now have extra information, such as negative value denotes insufficient funds (that it the price check failed).

I don't think you need a special check for price == CreditCheck.  Nice of you to throw in something extra, but why not tell them "Approved.  Remaining funds = ?".

You use decent indentation and nice that braces are wrapped around one line of code.

Why fix the credit to 8000?  Again, pass it as an argument.  If you do that, you should see why it's wrong that you hardcode it with:

WriteLine("Your credit limit is $8,000.00.\n");

I get that you want a blank line to appear after that.  For some people "\n" is perfectly acceptable.  Others would recommend using Environment.NewLine.  Putting many suggestions together, and using String Interpolation, this would become:

Console.WriteLine($"Your credit limit is ${creditLimit.ToString("N2")}.{Environment.NewLine}");

Or you could issue a simple Console.WriteLine(); to avoid the whole "\n" versus NewLine debate.


Timing single operation to not be repeated for a fixed time Sometimes "a" service does not respond and so we need to restart it. Usually it's a glitch in the network. We can have like 100 calls at the same time so the service cannot be restarted for 100 hundred times. The service is a Singleton. The command is called through a dispatcher, it's sync so different calls are executed in sequence.

Do you think that this approach is right to limit the restarting of the service?

public class RecoverServiceCommand extends AbstractCommand {
    /**
     * Logger for this class
     */
    private static final Logger log = LoggerFactory
        .getLogger(RecoverServiceCommand.class);

    @Override
    public boolean isAsynch() {
    return false;
    }

    @Override
    public boolean postCondition(Object... arg0) {
    return true;
    }

    @Override
    public boolean preCondition(Object... arg0) {
    return true;
    }

    @Override
    public void undo() {
    }

    @Override
    protected void doExecute(Object... arg0) throws CommandException {
    long now = System.nanoTime();
    synchronized (RecoverServiceCommand.class) {
        if (now - Service.getInstance().getLastUpdate() > Utils.TimeSpan)
        Service.getInstance().restartService(now);
        else
        log.warn("Messaging queues not restarted because it was already restarted not so long ago");
    }
    }

}

First of all, it looks like as a workaround for another bug. You might want to fix that bug instead of this workaround.
Comments like the following are unnecessary:

 /**
 * Logger for this class
 */

It just repeats the code and forces readers to read more than necessary.
Service.getInstance() is repeated. Create a local variable for it.
Synchronizing on public objects (like RecoverServiceCommand.class) is error-prone.  I'd use a private lock object instead. See: Avoid synchronized(this) in Java?
A minor change could be extracting out a boolean needsRestart local variable which would make the intention of the expression a little bit more clear.

Reference: Chapter 6. Composing Methods, Introduce Explaining Variable in Refactoring: Improving the Design of Existing Code by Martin Fowler

  Put the result of the expression, or parts of the expression,
  in a temporary variable with a name that explains the purpose.

Currently the code is tightly coupled with the Service and the Utils classes. If you want to write unit tests it's not too easy because it needs a working Service instance and it depends on the system time too. To make it easier to test you could pass the dependencies to the constructor:

private static final ReentrantLock lock = new ReentrantLock();

private final Service service;

private final Ticker ticker;

private final long updateInterval;

public RecoverServiceCommand(final Service service, final Ticker ticker,
        final long updateInterval) {
    this.service = checkNotNull(service);
    this.ticker = checkNotNull(ticker);
    checkArgument(updateInterval > 0, "updateIntarval must be bigger than zero");
    this.updateInterval = updateInterval;
}

@Override
protected void doExecute(final Object... arg0) throws CommandException {
    final long now = ticker.read();
    lock.lock();
    try {
        final boolean needsRestart =
            (now - service.getLastUpdate()) > updateInterval;
        if (needsRestart) {
            service.restartService(now);
        } else {
            log.warn("Messaging queues not restarted because "
                + "it was already restarted not so long ago");
        }
    } finally {
        lock.unlock();
    }
}

Here is a few tests with EasyMock:

import static org.easymock.EasyMock.expect;

import org.easymock.EasyMockSupport;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import com.google.common.base.Ticker;

public class RecoverServiceCommandTest {

    private EasyMockSupport mockery;
    private Service service;
    private Ticker ticker;

    @Before
    public void setUp() {
        mockery = new EasyMockSupport();
        service = mockery.createMock(Service.class);
        ticker = mockery.createMock(Ticker.class);
    }

    @After
    public void tearDown() {
        mockery.verifyAll();

    }

    @Test(expected = IllegalArgumentException.class)
    public void testInvalidUpdateInterval() throws Exception {
        final long invalidUpdateInterval = 0;

        mockery.replayAll();

        new RecoverServiceCommand(service, ticker, invalidUpdateInterval);
    }

    @Test
    public void testUpdateIsRequired() throws Exception {
        final long updateInterval = 100;

        final long currentTime = 300L;
        expect(ticker.read()).andReturn(currentTime);
        expect(service.getLastUpdate()).andReturn(199L);
        service.restartService(currentTime);

        mockery.replayAll();

        final RecoverServiceCommand command =
            new RecoverServiceCommand(service, ticker, updateInterval);
        command.doExecute();
    }

    @Test
    public void testUpdateIsNotRequired() throws Exception {
        final long updateInterval = 100;

        final long currentTime = 300L;
        expect(ticker.read()).andReturn(currentTime);
        expect(service.getLastUpdate()).andReturn(200L);

        mockery.replayAll();

        final RecoverServiceCommand command =
            new RecoverServiceCommand(service, ticker, updateInterval);
        command.doExecute();
    }

}

I've used the Ticker class from Guava. It's easy to mock it and it has a systemTicker which you can use in production code. The check methods are also in Guava.


Maintaining session timeout globally in java spring configuration Below is my code to maintain a session timeout globally in Java spring configuration and to load the session timeout value from a properties file. Do you have any suggestions to improve my code?

public class MySessionListener implements HttpSessionListener {
    @Override
        public void sessionCreated(HttpSessionEvent event) {
            try {
                ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
                InputStream stream = classLoader.getResourceAsStream("bimspring.properties");
                Properties properties = new Properties();
                properties.load(stream);
                String sessionTimeout = properties.getProperty("cookieName", "No Value Found");
                event.getSession().setMaxInactiveInterval(Integer.parseInt(sessionTimeout));
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

        @Override
        public void sessionDestroyed(HttpSessionEvent event) {

        }

    }

Quite a number of points for this short piece of code:

As the properties file will probably not change at runtime, loading it every single time a session gets created is a total waste. See whether you can cache the session timeout in a static variable.
If the value is not set, the default "no value found" will lead to a number format exception. Better use a sensible parseable default.
Similar context: by setting the value to a non-parseable string, you can kill the server with a misconfiguration.
"sessionTimeout" should not be named "cookieName"
Separation of concerns: a method should do one thing. This is a mixup of reading the program configuration and using the determined value to set the timeout.
Error handling: printStackTrace does not accomplish anything. Do something real (e.g. log an error for the server admin, shutdown, whatever.) If you decide to continue the process in case of an I/O exception, set a sensible default on the session.


What's the best name for a service object in Rails? One challenge I regularly face is how to name my service objects in Rails.

I like the advice in this blog post on Gourmet Service Objects, a service object performs one action. It holds the business logic to perform that action. For example:

# app/services/accept_invite.rb
class AcceptInvite

  def self.call(invite, user)
    invite.accept!(user)
    UserMailer.invite_accepted(invite).deliver
  end

end

However, I frequently find that my class names are rarely this short.

For example, I have areas that have many runners that have many goals.  I want to calculate, for a given area, the percentage of runners with a goal set.

Adhering to the above idea, that a service object performs one action, I've come up with the following:

# app/services/get_percentage_of_runners_with_goals.rb
class GetPercentageOfRunnersWithGoals

  def self.call(area)
    membership_count = area.runners.members.to_float
    goals_set_count = area.runners.with_goals.to_float
    percentage = goals_set_count / membership_count
    (percentage * 100).to_s(:percentage, precision: 0)
  end

end

What then tends to happen is I have hundreds of these service objects, named along the lines of VerbObjectForObject, i.e. GetMembershipStatusOfRunner, GetCurrentGoalOfRunner, SetGoalOfRunner, and so on.

Is there a better naming convention?

You could use modules to group them which would potentially shorten the names too. You'd probably need to look at the types of service methods you have before deciding how to group them (e.g. by model: Runner::MembershipStatus or activity: Statistics::RunnersWithGoals).

Presumably you've already decided you don't want these methods on your models? For me a service method makes most sense if it's performing an action rather than just retrieving some information. So GetCurrentGoalOfRunner would probably sit on my Runner model as runner.current_goal rather than being a separate service.

Possibly also worth looking at Mutations (which I was introduced to recently): https://eng.uservoice.com/blog/2013/02/27/introducing-mutations-putting-soa-on-rails-for-security-and-maintainability/


Reading, writing and filtering a CSV file I have a CSV with 5+ million rows and I want to filter it:

with open("test1.csv") as csv_file:
    writer =csv.writer(outputfile, delimiter=',')
    for row in csv.reader(csv_file, delimiter=','):
        row = [int(x) for x in row]

        for i in range (1,11):
            if add_i (row) == True:
                break

            elif row not in (row_dup):
                row_dup.append(row)
                writer.writerow(row)

I have many functions, the simplest one being add_i which you can see in the snippet above.

if row[0:3] == range(row[0], row[0] +3 * i, i):
    return True

row_dup = [] is an empty array I use to store duplicates so that they don't get written to the file.

I want to improve the execution time of my script, but it takes hours. According to the calculation of a friend, it takes 530 hours or so. I'm not sure about that but regardless, it is still taking a long time.

How I tried to improve the speed of the script:

Using PyPy but I didn't notice a significant improvement.
Removing row = [int(x) for x in row] but then I'd have to write if [int(x) for x in row[0:4]] == range(int(row[0]), int(row[0]) + 4 * i, i):
it's annoying and there's still a for loop.
Removing for i in range (1,11) but then I would have to write the indexes myself.
Removing for i in range (1,11) and row = [int(x) for x in row] and doing it like so

if (int(row[0]) + 1 == int(row[1])) and int(row[1]) + 1 == int(row[2])
and int(row[2]) + 1 == int(row[3]) and int(row[3]) + 1 == int(row[4]):
    do something

This is a desperate plan and the most annoying. I get rid of two for loops and do everything by hand, increasing the index value from 1 to 11 by copy pasting.
Considered using Hadoop but the VPS I have are so much less powerful than my PC, low end box.
Considered using SQLite instead of CSV but not convinced it would make things much faster.
Considered writing it in C++, and in fact the first version was in C++ but moved to Python since I didn't notice a big difference and because it's easier to plot in Python.

And by the way, I'm on Manjaro 64 bit, filesystem Ext4, Python 2.7, running in virtualenv. I'm only using a CSV module. I have an HP Pavilion G6 laptop, if you want to see the specs of it.

Note that the code is taking that much time, even though I'm only testing with add_i so I didn't even bother testing the complicated function. Who knows how much time it would take then.

This is desperate plan and the most annoying, I get rid of 2 for loops and do everything by hand

I will take this question to be 'How can I remove the two inner for loops if possible?'.

First off, you change the row slice from 'the first three' to 'the first four' in your question. And so post all your code. Don't make us guess.
Anyway, this is what I think you want to give us:

import csv

def add_i(row, i):
    if row[0:3] == range(row[0], row[0] +3 * i, i):
        return True

row_dup = []

with open("out_file", "w") as out_file:
    writer =csv.writer(out_file, delimiter=',')
    with open("test1.csv") as csv_file:
        for row in csv.reader(csv_file, delimiter=','):
            row = [int(x) for x in row]

            for i in range (1,11):
                if add_i (row, i) == True:
                    break

                elif row not in (row_dup):
                    row_dup.append(row)
                    writer.writerow(row)

First, we don't want this to be the worst version of cp, and so we will use the for else statement.

for i in range(1,11):
    if add_i(row, i):
        break
else:
    if row not in row_dup:
        # ...

This makes it so that [2, 4, 6] gets removed as well. This is as you would have a bug, so that in add_i, range(2, 5, 1) would return False, and add it to the output.

we can make the for statement, just an if statement. This is as you use range(start, stop, step). I will use i, j and k for start, stop and step respectfully.
We can say that range returns this sort of structure [i, i+k, i+2k, ... i+((j-i)//k)*k]. As you use range(row[0], row[0] + 3*i, i) we know that is [i, i+k, i+2k] where i and k are row[0] and i respectfully.
As this is getting rather complicated. Here is how I would implement it.

def add_i(row, range):
    step = row[1] - row[0]
    if range[0] <= step < range[1]:
        return step == (row[2] - row[1])
    return False

This while not comparing lists uses the logic, as the difference between them all has to be step, and step has to be in the allowed range.

Now that we have gotten rid of one of the two internal for loops, we need to get rid of the other, for this to be what I would deem is the 'correct' answer.
This loop is a list comprehension, and if you have only 3 columns in all the rows, it would be a relatively fast and a really easy conversion. However, I am guessing that is not the case.
And so if we just remove it, we will have to manually convert the columns into numbers. We can just do this in add_i again.

def add_i(row, range):
    r1 = int(row[1])
    step = r1 - int(row[0])
    if range[0] <= step < range[1]:
        return step == (int(row[2]) - r1)
    return False

Edit, I thought I should make a benchmark for sets, and found it was very fast.

row_dup = set()

# ... in the for loop

row_ = ''.join(row)
if not add_i(row, (1, 11)) and row_ not in row_dup:
    row_dup.add(row_)

This allows for very fast look-ups. And only has a ~27% impact on speed. The way that we make it so that we can use sets, is by making the data hash-able. Strings are hash-able, and with the change in the program, where the data in the row is all strings, it allows you to simply str.join it all together.

And so we have an answer with only one for loop.

import csv

def add_i(row, range):
    r1 = int(row[1])
    step = r1 - int(row[0])
    if range[0] <= step < range[1]:
        return step == (int(row[2]) - r1)
    return False

row_dup = set()

with open("out_file", "w") as out_file:
    writer = csv.writer(out_file, delimiter=',')
    with open("test1.csv") as csv_file:
        for row in csv.reader(csv_file, delimiter=','):
            row_ = ''.join(row)
            if not add_i(row, (1, 11)) and row_ not in row_dup:
                row_dup.add(row_)
                writer.writerow(row)

This is roughly O(n). And you can add a list comprehension to allow more steps easily.

def add_i(row, range):
    r0 = int(row[0])
    step = int(row[1]) - r0
    if range[0] <= step < range[1]:
        return row[1:3] == [str(i) for i in range(r0 + step, r0 + 3*step, step)]
    return False

As you seem to want to look over more than just 3 columns, due to your example with four, using a list comprehension is probably the way how you would want to do this.

If you expand on this then you would want to change add_i to only take a row, and be called something like format_test. This way, you are testing if the row follows a format, and if it does not then you add it, if not already added, to the new file. You can also now tell what the main part of the function is doing better, as it is saying, 'if this row passes this test, add it to the out file.'

Also, if you want some benchmarks.
I used 100000 random 3 column csv lines, of numbers up to 10000.

New program with row_dup as type list:

real    3m37.109s
user    3m36.857s
sys     0m0.057s

New program with row_dup as type set:

real    0m0.453s
user    0m0.440s
sys     0m0.010s

New program without row_dup:

real    0m0.357s
user    0m0.350s
sys     0m0.000s

Old program without row_dup:

real    0m1.321s
user    0m1.290s
sys     0m0.010s


Is it bad practice to increment more than one variable in a for loop declaration? While coding today, I came across the following loop in C#:

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++, count++){
        some2DarrayRepresentedBy1D[count] = ...;
    }
}

Would this be consider bad practice?  Would the following be a better way to achieve the same result:

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++){
        some2DarrayRepresentedBy1D[count] = ...;
        count++;
    }
}

Would you allow both through, for example, a code review or would you tag the first example for being obscure?

I would agree with the OP in that the second code block would be the best approach.

Reason - readability.   In my opinion, the variable incremented in the for loop statement should only be used for keeping count of how many times we're looping.   count might be set before the loop, it might also be incremented / decremented in several different loops.   But in either case, count isn't used to keep track of the progress of the current loop - y is in this case.

So, I would say that it's best practice, for readability sake, to only increment the variable used to loop through the for loop in the for loop line itself.


A Slice-type for C++ The Go programming language has that slice type, which is under the hood a pointer to an actual array. This allows treating the subarrays as actual arrays with almost no performance overhead. This code snippet is about implementing slice type for C++ arrays and vectors.

slice.h:

#ifndef SLICE_H
#define SLICE_H

#include <sstream>
#include <stdexcept>
#include <string>

template<class T>
class slice {
    private:
        T*     m_array;
        size_t m_length;

    public:
        slice(T* array, size_t length) {
            this->m_array  = array;
            this->m_length = length;
        }

        slice(T* array, size_t length, size_t skip) {
            this->m_array = array + skip;
            this->m_length = length;
        }

        T& operator[](size_t index) {
            if (index >= m_length) {
                std::ostringstream os;
                os << "Bad index: " << index << ", slice size: " << m_length;
                throw std::runtime_error(os.str());
            }

            return m_array[index];
        }

        size_t size() {
            return m_length;
        }

        T* begin() {
            return m_array;
        }

        T* end() {
            return m_array + m_length;
        }

        const T* begin() const {
            return m_array;
        }

        const T* end() const {
            return m_array + m_length;
        }
};

#endif  /* SLICE_H */

main.cpp:

#include <algorithm>
#include <iostream>
#include <vector>

#include "slice.h"

using std::cout;
using std::endl;
using std::vector;

int main(int argc, char** argv) {
    int int_array[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };
    //// Should be 4, 5, 6, 7, 8.
    slice<int> pizza_slice = slice<int>(int_array, 5, 3);

    cout << "Arrays slice size: " << pizza_slice.size() << endl;

    for (int i : pizza_slice)
    {
        cout << i << endl;
    }

    vector<int> int_vector = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };

    //// Should be 2, 3, 4
    pizza_slice = slice<int>(&int_vector[0], 3, 1);

    cout << "Vector slice size: " << pizza_slice.size() << endl;

    for (int i : pizza_slice)
    {
        cout << i << endl;
    }

    return 0;
}

Any critique is much appreciated.

This looks pretty good.

Use mem-initializer lists

When you write

slice(T* array, size_t length) {
    this->m_array  = array;
    this->m_length = length;
}

you're default constructing m_array and m_length and then assigning them. For pointers and integral types, this isn't exactly expensive, but it's a waste and may as well not get into that habit. Prefer directly constructing:

slice(T* array, size_t length)
: m_array(array)
, m_length(length)
{ }

Your other constructor can on the one hand just delegate to this one:

slice(T* array, size_t length, size_t skip)
: slice(array + skip, length)
{ }

but on the other hand doesn't really need to exist anyway. Just let the user pass in where he wants to start. This strikes me as just bloaty.

const functions

size() should be const, and you're missing T const& operator[](size_t) const.

You could also provide a T* data() and T const* data() const, which in this case just alias begin().

Additionally, consider providing a different constructor so that I can construct a slice<const T> from a slice<T>. That's something that makes sense.

throwing

If you're throwing due to out of range indexing, you should throw std::out_of_range. But generally, avoid having operator[] throw. This is what you're going to use most often so it's best to make it as simple as possible. If you want to provide a throwing alternative, the typical pattern followed by the standard library is to provide an at() function that does the bounds checking and then forwards along to operator[].

constexpr

Everything here can be constexpr. Just go overboard.


Applying same formatting to multiple borders in Excel VBA? Is there a better way to format the cells with borders than what you get when you record a macro?  For example, I want to add borders to a cell range.  The recorded code is:

Range("A1:C19").Select
Selection.Borders(xlDiagonalDown).LineStyle = xlNone
Selection.Borders(xlDiagonalUp).LineStyle = xlNone
With Selection.Borders(xlEdgeLeft)
    .LineStyle = xlContinuous
    .ColorIndex = 0
    .TintAndShade = 0
    .Weight = xlThin
End With
With Selection.Borders(xlEdgeTop)
    .LineStyle = xlContinuous
    .ColorIndex = 0
    .TintAndShade = 0
    .Weight = xlThin
End With
With Selection.Borders(xlEdgeBottom)
    .LineStyle = xlContinuous
    .ColorIndex = 0
    .TintAndShade = 0
    .Weight = xlThin
End With
With Selection.Borders(xlEdgeRight)
    .LineStyle = xlContinuous
    .ColorIndex = 0
    .TintAndShade = 0
    .Weight = xlThin
End With
With Selection.Borders(xlInsideVertical)
    .LineStyle = xlContinuous
    .ColorIndex = 0
    .TintAndShade = 0
    .Weight = xlThin
End With
With Selection.Borders(xlInsideHorizontal)
    .LineStyle = xlContinuous
    .ColorIndex = 0
    .TintAndShade = 0
    .Weight = xlThin
End With

Can this be shortened?

If the question is "how do I apply the same formatting to multiple borders?" then the following is one way:

Dim the_borders As Variant
the_borders = Array(xlEdgeLeft, xlEdgeTop, xlEdgeBottom, xlEdgeRight, xlInsideVertical, xlInsideHorizontal)
    ' Or whatever xlEdge* constants you want to list

Dim idx As Long
For idx = LBound(the_borders) To UBound(the_borders)
    With Selection.Borders(the_borders(idx))     ' Process the right border
        .LineStyle = xlContinuous
        .ColorIndex = 0
        .TintAndShade = 0
        .Weight = xlThin
    End With
Next idx

The As Variant / Array combo gives you an array of the border IDs you specify, running from LBound(the_borders) to UBound(the_borders).  Then, within the loop, borders(idx) is the XlBordersIndex you can pass to Selection.Borders.


Convert Iterable tree to string I code this function to convert list of dict to a specific string to export last in a row table string field.

#!/usr/bin/env python
# encoding: utf-8
from __future__ import unicode_literals, print_function
import types
import collections
from collections import Iterable

def convert_to_string(input):
    if isinstance(input, dict):
        return "{{{}}}".format(",".join(list("{}:{}".format(convert_to_string(key), convert_to_string(value)) for key, value in input.iteritems())))
    elif isinstance(input, list, tuple):
        return "[{}]".format(",".join(list(convert_to_string(element) for element in input)))
    elif isinstance(input, types.StringTypes):
        if isinstance(input, types.UnicodeType):
            return input
        if isinstance(input, types.StringType):
            return input.encode("utf8")
    elif isinstance(input, types.NoneType):
        return ""
    elif isinstance(input, (types.IntType, types.LongType, types.FloatType)):
        return "{}".format(input)
    else:
        try:
            return "{}".format(input)
        except:
            return "<Undefined>"

data = [[{"1": "pomme", "2": "poiré"}, u"pétrole"], [None, 4], [5.00, 6], 7, Iterable]
print (convert_to_string(data))

That return: in IPython

convert_to_string(data)
Out[18]: u"[[{1:pomme,2:poir\xe9},p\xe9trole],[,4],[5.0,6],7,<class '_abcoll.Iterable'>]"
print(convert_to_string(data))
[[{1:pomme,2:poiré},pétrole],[,4],[5.0,6],7,<class '_abcoll.Iterable'>]

Function work fine but do you know more efficient solution to do that?

I fail to see if the code actually worked, because the isinstance should raise an exception:

    elif isinstance(input, list, tuple):
TypeError: isinstance expected 2 arguments, got 3

The second argument should be a tuple of types:

    elif isinstance(input, (list, tuple)):

but that might be a simply copy-over error.

In your code you do not need to force conversion to a list. The .join function can take an iterator as its argument. You free up a little memory for your program execution.

if isinstance(input, dict):
    return "{{{}}}".format(",".join(("{}:{}".format(convert_to_string(key), convert_to_string(value)) for key, value in input.iteritems())))
elif isinstance(input, list, tuple):
    return "[{}]".format(",".join((convert_to_string(element) for element in input)))

Since you already have imported the types module, I'd suggest that you be consistent, and use types.ListType, types.DictType etc. as well.


Rust Beginner: flatten nested matches This is how I implemented the guessing game - chapter 2 of the Rust book: https://doc.rust-lang.org/book/ch02-00-guessing-game-tutorial.html
use std::io;
use std::io::Error;
use std::cmp::Ordering;
use std::num::ParseIntError;

use rand::Rng;

enum MyReadLineError {
    FailReadLine(Error)
}

#[derive(Debug)]
enum MyReadU32Line {
    FailReadLine(Error),
    FailParse(ParseIntError)
}

fn my_read_line() -> Result<String, MyReadLineError> {
    let mut input = String::new();
    let result = io::stdin().read_line(&mut input);
    match result {
        Ok(_) => Ok(input),
        Err(error ) => Err(MyReadLineError::FailReadLine(error)),
    }
}

fn my_read_u32_line() -> Result<u32, MyReadU32Line> {
    match my_read_line() {
        Result::Ok(line) =>
            match line.trim().parse::<u32>() {
                Ok(value) => Ok(value),
                Err(error) => Err(MyReadU32Line::FailParse(error)),
            }

        Result::Err(MyReadLineError::FailReadLine(error)) => Err(MyReadU32Line::FailReadLine(error)),
    }
}

fn main() {
    println!("Guess the number!");

    let secret_number = rand::thread_rng().gen_range(1, 101);

    loop {
        println!("Please input your guess.");
        let guess = my_read_u32_line();
        match guess {
            Ok (value) => match value.cmp(&secret_number) {
                Ordering::Less => println!("Too small!"),
                Ordering::Greater => println!("Too big!"),
                Ordering::Equal => {
                    println!("You win!");
                    break;
                },
            }
            Err (_) => println!("Sorry I couldn't read a u32 from your input! Please try again."),
        }
    }
}

This works as expected but I would like to flatten the matches so that doesn't look at verbose, is that possible. Also is there any other way to make that code look better, more readable?

Welcome to Rust!
Code Formatting
There are some inconsistencies in your formatting — I have run
cargo fmt to get rid of them.
Naming
There's no need to prefix everything with My or my_ — in
Rust, your names won't as easily clash with existing names as might be
the case in certain other languages.
Error handling
For applications, I suggest using the anyhow crate to manage
errors — it reduces much of the boilerplate.  Just add
anyhow = "1.0"

to the [dependencies] section of your Cargo.toml.
Helper functions
The my_read_line function can be simplified with the ? operator:
use anyhow::Result;

fn read_line() -> Result<String> {
    let mut input = String::new();
    io::stdin().read_line(&mut input)?;
    Ok(input)
}

my_read_u32_line can be similarly simplified:
fn read_u32() -> Result<u32> {
    Ok(read_line()?.trim().parse()?)
}

Magic numbers
We can define constants for the range of the number:
const MIN: u32 = 1;
const MAX: u32 = 100;

// later
let secret_number = rand::thread_rng().gen_range(MIN, MAX + 1);

Organization
I might reorganize the main function to reduce indentation:
loop {
    println!("Please input your guess.");

    let guess = match read_u32() {
        Ok(guess) => guess,
        Err(_) => {
            println!(
                "Sorry I couldn't read a u32 from your input! \
                Please try again."
            );
            continue;
        }
    };

    match guess.cmp(&secret_number) {
        Ordering::Less => println!("Too small!"),
        Ordering::Greater => println!("Too big!"),
        Ordering::Equal => {
            println!("You win!");
            break;
        }
    };
}

Final result
use anyhow::Result;
use rand::Rng;
use std::cmp::Ordering;
use std::io;

const MIN: u32 = 1;
const MAX: u32 = 100;

fn read_line() -> Result<String> {
    let mut input = String::new();
    io::stdin().read_line(&mut input)?;
    Ok(input)
}

fn read_u32() -> Result<u32> {
    Ok(read_line()?.trim().parse()?)
}

fn main() {
    println!("Guess the number!");

    let secret_number = rand::thread_rng().gen_range(MIN, MAX + 1);

    loop {
        println!("Please input your guess.");

        let guess = match read_u32() {
            Ok(guess) => guess,
            Err(_) => {
                println!(
                    "Sorry I couldn't read a u32 from your input! \
                    Please try again."
                );
                continue;
            }
        };

        match guess.cmp(&secret_number) {
            Ordering::Less => println!("Too small!"),
            Ordering::Greater => println!("Too big!"),
            Ordering::Equal => {
                println!("You win!");
                break;
            }
        };
    }
}


Manager for image and JSON assets of a game I would like to know what you think of this assets manager for a game. If it can be optimized or simplified? It allows you to download sprites, tiles, json files, and other images.

const images = {};
images.tiles = {};

exports.images = images;
/**
* Download an image and store it in the cache
* @param {string} url - Address of the image
* @param {string} name - Name of the image
* @param {function} onImageLoaded -Execute this function when all images are loaded
*/

const loadImage = (url, name, onImageLoaded) => {

    if (images[name]) return onImageLoaded(images[name]);

    const image = new Image();

    image.onload = () => {
        if (image.complete) {
            images[name] = image;
            return onImageLoaded(image);
        }
    };

    image.src = url;
};

/*
* Download all images
* @param {object} images -
* @param {function} onImagesLoaded -
*/

const loadImages = (images, onImagesLoaded) => {
    var imagesLoaded = 0;

    const onImageLoaded = () => {
        imageLoaded++;

        if (images.length != imagesLoaded) return ;
        onImagesLoaded();
    }

    for (const image of images)
        loadImage(image.url, image.name, onImageLoaded)
};

/*
* Download all tiles
* @param {object} tiles - Tiles to download
* @param {function} onTilesLoaded - Function that is executed when the tiles are downloaded
*/

const downloadTiles = (tiles, onTilesLoaded) => {

    const images = tiles.map(tile => {
        const image = {};

        image.name = 'tile_' + tile.id;
        image.url =  'assets/tiles/' + image.name + '.png';

        return image;
    });

    loadImages(images, onTilesLoaded);
};

exports.downloadTiles = downloadTiles;

/*
* Download all the images and store them in cache
* @param {object} sprites - Sprites to download
* @param {function} onSpritesLoaded - Function that is executed when the sprites are downloaded
*/

const downloadSprites = (sprites, onSpritesLoaded) => {
    const images = sprites.map(sprite => {
        const image = {};

        image.name = 'sprite_' + sprite.id;
        image.url = 'assets/sprites/' + image.name + '.png';

        return image;
    })

    loadImages(images, onSpritesLoaded);
}

const toJSON = response => response.json();

/*
* @param {string} url -
* @param {string} name -
*/

const loadJSON = (url, name) =>
    fetch(url, { method: 'GET' })
        .then(toJSON);

exports.loadJSON = loadJSON;

exports.loadImage = loadImage;
exports.loadImages = loadImages;

const getTileCoordinates = (image, index, width, height) => {
    var row = 0;
    var x = width * index, y;

    while(true){
        if (x < image.width) break ;
        x-=image.width;
        row++;
    };
    y = height * row;

    return {x, y};
};
exports.getTileCoordinates = getTileCoordinates;

Naming and Commenting
An important part of being a programmer is good naming.
Looking at your code at first glance it seemed fine, until I started to rewrite it (I always rewrite when reviewing). For good code I should be able to rewrite the code without the need to go back to the original source for clarification. In this example I was constantly going back to try and understand what you were doing.
The main reason was that the name of variables that did not match what the variables contained and conflicted with important global names.
Comments
To make it worse the comments confused things even further
So let's examine the comments

/**
* Download an image and store it in the cache
* @param {string} url - Address of the image
* @param {string} name - Name of the image
* @param {function} onImageLoaded -Execute this function when all images are loaded

Last line is completely wrong, maybe?
  /* @param {function} onImageLoaded - callback for image onload event

3rd line not helpful , maybe?
 /* @param {string} name - Name used to store image in images object

2nd line, address? its a url or more apt a relative path to the image resource
1st line store it in a cache? Can't find any cache, but looking at the code I found the images object which you could call a cache, and download no you are not downloading in the classical sense of the word.
Maybe the following more clearly describes what the function does.
  /* Creates and loads Image adding it as a named object to images

The the second function's comments are as clear as mud

/* Download all images
 * @param {object} images -
 * @param {function} onImagesLoaded -

maybe
 /* Create and load images from an array image descriptions
  * @param {object} images - array of image descriptions contains url and name
  * @param {function} onImagesLoaded - Callback function called when all
  *                                    images in array have loaded.

I have to admit that the very first thing I do when reviewing code is remove all comments (they rarely help), so your comments did not confuse me, only when writing this did I read them.
If you are going to add comments they must make sense, for if they dont they only create confusion, and that will result in problems when you return in months to touch up the code.
Write comments addressed to a 3rd person that is not in your head space. Sure the images object is a cache, you know that but that is not obvious to anyone else.
Names
Names are short.
When you name a variable it is at the point where you define its scope, that scope gives the variable a context. You name a variable in its context and don't add redundant information that is clearly implied from the context.
For example you have the argument onImageLoaded for the function loadImage I would say that the 'Image' part of the name is rather obvious so why add it. onLoaded is shorter, does not create ambiguity.
Names create entities
When you name an object you create an entity in the mind of the person reading your code (that person will be you months from now). When you see that name you associate the entity. Associating a global name with multiple entities leads to confusion.
Global names should be unique across its scope. You used the name images  defined as const images = {}; but in parts of the code you also use the name images to represent an array. So you will have to be checking is this the images in global scope, or the images as an array.
Also image is used to represent an Image in some places and an image description in other places, confusing...
General comments

Use strict mode it will result in better code and runs faster which is good for games. Add "use strict"; to the top of the JS file.

Don't forget semicolons

Don't create blockless blocks

//bad
for (const image of images)
    loadImage(image.url, image.name, onImageLoaded)

// good
for (const image of images) {                         // define block start
     loadImage(image.url, image.name, onImageLoaded); // semicolon
}                                                     // define block  end

You are not using standard export so that means the whole set of functions could be in global scope. Use a singleton or use the native export. Try to limit your exports, exporting every function is just a pain an will lead to problems.

The function getTileCoordinates is out of place and does not belong here. BTW there is a better way to find the row

function getTileCoordinates(image, index, width, height){
    const tilesAcross = image.width / width | 0; // | 0 floors result
    return {
        x : width * (index % tilesAcross),
        y : (index / tilesAcross | 0) * height
    };
}

Don't repeat. The functions downloadSprites and downloadTiles are repeats

I could not work out why you are returning the result of the load callbacks. There is no consistency as you do it sometimes and other times not. Also you are passing the image as an argument to the callbacks but there is no indication that you use it. For the onload event you can access the image via this if needed. Note you will have to use a standard function declaration for the callback.

There is no error checking. What do you do if the connection or a request is lost?

A rewrite
This is how I would have written your code. It is just a suggestion example and far from ideal. I do not know what you wanted exposed but I only exposed what was not called internally (assuming this is a module)
"use strict";
const images = {
    tiles : {},
};

function loadImage(imageDesc, onLoad) {
    var image = images[imageDesc.name];
    if (image) { onLoad() }
    else{
        image = images[imageDesc.name] = new Image();
        image.src = imageDesc.url;
        image.onload = onLoad;
    }
},

function loadImages(imageArray, onAllLoaded) {
    var imagesLoaded = 0;
    const onLoad = () => {
        if (imageArray.length === ++ imagesLoaded) { onAllLoaded() }
    }
    for (const imageDesc of imageArray) { loadImage(imageDesc, onLoad) }
},

function loadList(type, list, onLoad) {
    loadImages(
        list.map(imageDesc => {
            const name = `${type}_${imageDesc.id}`;
            return  { name, url : `assets/${type}/${name}.png` };
        }),
        onLoad
    );
},

const assests = {
    images,
    loadTiles(tiles, onLoaded) {
        loadList("tiles", tiles, onLoaded);
    },
    loadSprites(sprites, onLoaded) {
        loadList("sprites", sprites, onLoaded);
    },
    loadJSON : (url, name) => fetch(url, { method: 'GET' }).then(response => response.json()),
};

export default assests;


Logger module with JavaScript I'm new to JavaScript, and in Java I probably would do this as a singleton. I need something that I can call from anywhere and instantiate only once.

This is what I have done, but I need some advice to write it in the right way in this new prototype based and functional JavaScript.

var winston = require('winston');

var genericLogger = null,
    dbLogger = null,
    viewLogger = null,
    networkLogger = null;

function initLogger() {
    console.log('logger.js > initLogger');

    genericLogger = new (winston.Logger)({
        transports: [
          new (winston.transports.Console)(),
          new (winston.transports.File)({ filename: './log/error.log' })
        ]
    });

    dbLogger = new (winston.Logger)({
        transports: [
          new (winston.transports.Console)(),
          new (winston.transports.File)({ filename: './log/db.log' })
        ]
    });

    viewLogger = new (winston.Logger)({
        transports: [
          new (winston.transports.Console)(),
          new (winston.transports.File)({ filename: './log/view.log' })
        ]
    });

    networkLogger = new (winston.Logger)({
        transports: [
          new (winston.transports.Console)(),
          new (winston.transports.File)({ filename: './log/network.log' })
        ]
    });
}

initLogger();
var Logger = {};

Logger.error = function(n) {
  genericLogger.log('error', n);
};

Logger.info = function(n) {
  genericLogger.log('info', n);
};

Logger.db = function(t, n) {
    if(n) {
        dbLogger.log(t, n);
    } else {
        dbLogger.info(t);
    }
};

Logger.view = function(t, n) {
    if(n) {
        viewLogger.log(t, n);
    } else {
        viewLogger.info(t);
    }
};

Logger.network = function(t, n) {
    if(n) {
        networkLogger.log(t, n);
    } else {
        networkLogger.info(t);
    }
};

module.exports = Logger;

I can call it from anywhere with require, and I expose only few public methods with module.exports and due to the require mechanism there is only one instance.

Is there a cleaner way to do it? More OOP? More functional?

Interesting question,

You have a ton of repeating code, so don't hesitate to write more helper functions so that this:

genericLogger = new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.File)({ filename: './log/error.log' })
    ]
});

dbLogger = new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.File)({ filename: './log/db.log' })
    ]
});

viewLogger = new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.File)({ filename: './log/view.log' })
    ]
});

networkLogger = new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.File)({ filename: './log/network.log' })
    ]
});

could be:

function createLogger( filename ){
  return new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.File)({ filename: filename })
    ]
  });
}
genericLogger  = createLogger( './log/error.log' );
dbLogger       = createLogger( './log/db.log' );
viewLogger     = createLogger( './log/view.log' );
networkLogger  = createLogger( './log/network.log' );

I did not test this code, but you get the gist.

Furthermore, in this code, I have no idea what t and n are:

Logger.db = function(t, n) {
    if(n) {
        dbLogger.log(t, n);
    } else {
        dbLogger.info(t);
    }
};

Logger.view = function(t, n) {
    if(n) {
        viewLogger.log(t, n);
    } else {
        viewLogger.info(t);
    }
};

Here as well, you could use a helper function:

Logger.loggerHelper = function( o , t , n ){
  if(n){
    o.log(t,n)
  } else {
    o.log(t)
  }
}

Logger.db = function(t, n) {
  loggerHelper( dbLogger , t , n );
};

Logger.view = function(t, n) {
  loggerHelper( viewLogger , t , n );
};

After a while I would get irritated by the above as well, and write something like this:

var winston = require('winston');

var Logger = {};
var loggers = {
  generic : './log/error.log',
  db : './log/db.log',
  view : './log/view.log',
  network : './log/network.log'
};

for( logger in loggers){

  Logger['_' + logger] =  new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.File)({ filename: loggers[logger] })
    ]
  });

  Logger[logger] = function(t,n ){
    if(n) {
        Logger['_' + logger].log(t, n);
    } else {
        Logger['_' + logger].info(t);
    }
  }

}

Logger.error = function(n) {
  Logger['_generic'].log('error', n);
};

Logger.info = function(n) {
  Logger['_generic'].log('info', n);
};

module.exports = Logger;

loggers contains the link between the method and the file name, so it becomes now very easy to add/remove/change loggers. This approach does expose a lot more (all) the functionality in Logger, but I think that would work out.


A script to remove unneeded files The hard drive in my dev box is running out of space so I wrote this little script to try and free up space until I get a new drive.

My Concerns

Is the code pythonic?
Does the code go against any best practices?

import glob
import os
import psutil
import shutil

PATHS = ['/var/log/*', '/tmp/*', '/home/ricky/Downloads/']

USAGE = psutil.disk_usage('/').percent

if os.geteuid() != 0:
    exit("You need to have root privileges to run this script.")

def flatten(alist):
    if alist == []:
        return []
    elif type(alist) is not list:
        return [alist]
    else:
        return flatten(alist[0]) + flatten(alist[1:])

if __name__ == '__main__':
    for path in flatten(map(glob.glob, PATHS)):
        if os.path.isfile(path):
            os.remove(path)
        if os.path.isdir(path):
            shutil.rmtree(path)

    print "Usage Before {}\nUsage After {}".format(USAGE, psutil.disk_usage('/').percent)

Example Output:

ricky@ricky-OptiPlex-330:~/Projects/Python
$ sudo python cleanup.py
Usage Before 83.5
Usage After 82.1

Move your code inside a function rather than doing things at module level to make it re-useable. Your function could accept a list of paths either through a function call or through command line arguments instead of a global list of paths.
Keep standard library, third party imports separate.
You could use itertools.chain.from_iterable instead of your flatten function. Also use glob.iglob to get an iterator instead of list of files to save memory.

for path in chain.from_iterable(glob.iglob(path) for path in PATHS):
    ...

For comparing types use isinstance instead of comparing types using is. is won't work in case of multiple-inheritance. Read: Differences between isinstance() and type() in python


Linear Regression Class in Python I have recently been brushing up on my statistics and calculus and wanted to implement Linear Regression, the code will be for a calculus/statistics library I am working on (I know there are libraries for this but I am trying improve my both coding and math skills).

The following code works as intended. However, I feel like it has some structural flaws and I can't put my finger on what it is

class LinearReg:

    x = []
    y = []

    x_mean = 0
    y_mean = 0

    b_zero = 0
    slope = 0

    def __init__(self,x,y):

        if len(x) != len(y):
            raise Error("Both axis must have the same number of values")

        self.x = x
        self.y = y
        self.x_mean = self.axis_mean(self.x)
        self.y_mean = self.axis_mean(self.y)

    def axis_mean(self,axis):
        return sum(axis) / len(axis)

    def sum_of_deviation_products(self):
        result = 0
        for i in range(len(self.y)):
            x_dev = (self.x[i] - self.x_mean)
            y_dev = (self.y[i] - self.y_mean)
            result += x_dev * y_dev
        return result

    def sum_of_x_deviation_squared(self):
        result = 0
        for i in range(len(self.x)):
            result += (self.x[i] - self.x_mean)**2
        return result

    def get_b_zero(self):
        return self.b_zero

    def get_slope(self):
        self.slope = self.sum_of_deviation_products() / self.sum_of_x_deviation_squared()
        return self.slope

    def fit_best_line(self):
        self.b_zero = self.y_mean - (self.get_slope() * self.x_mean)
        print("The best line equation is: " +
        "%.2f" % self.slope
         + "x + " +
         "%.2f" % self.b_zero)

My primary focus is now: structure and cleanliness,

How efficient would this play out in a large library?
How clean and readable is the code?
What kind of problems would I face if this class were to interact with other classes in the library?

Perhaps there is quicker way to do Linear Regression in Theory, but I'm not really interested in that right now.

Toward optimized functionality and design

since all crucial attributes x, y, x_mean, y_mean are initialized on LinearReg instance creation (within __init__ method) - no need to duplicate and define them as class attributes (x = [] ... y = []), those should be removed as redundant
raise Error - Error is not Python exception class. Use Exception or ValueError
axis_mean(self, axis) function does not use self context and deserves to be just a @staticmethod
sum_of_deviation_products/sum_of_x_deviation_squared functions
Substitute algorithm: instead of going with external variable result + range(len(..)) + for loop - apply a convenient combination of sum + enumerate functions:

def sum_of_deviation_products(self):
    return sum((self.x[i] - self.x_mean) * (y - self.y_mean)
            for i, y in enumerate(self.y))

A good (or better) alternative would be sum + zip approach: sum((x - self.x_mean) * (y - self.y_mean) for x, y in zip(self.x, self.y))
get_b_zero method is redundant as it's just return a public attribute self.b_zero (which is accessed directly)
get_slope method
Instead of storing and reassigning self.slope attribute on each method invocation - as it's recalculated each time, it deserves to be a computed attribute using @property decorator:

@property
def slope(self):
    return self.sum_of_deviation_products() / self.sum_of_x_deviation_squared()

Now, it's simply accessed with self.slope.
fit_best_line method
Instead of unreadable string concatenation like ... + "%.2f" % self.slope + "x + " + "%.2f" % self.b_zero use flexible f-string formatting:

print(f"The best line equation is: {self.slope:.2f}x + {self.b_zero:.2f}").

The conciseness and flexibility are obvious.
Also, in case if b_zero attribute/property happen to be only actual in context of fit_best_line call - it can be eliminated and declared as just local variable.

The final optimized approach:

class LinearReg:
    def __init__(self, x, y):
        if len(x) != len(y):
            raise ValueError("Both axis must have the same number of values")

        self.x = x
        self.y = y
        self.x_mean = self.axis_mean(self.x)
        self.y_mean = self.axis_mean(self.y)
        self.b_zero = 0

    @staticmethod
    def axis_mean(axis):
        return sum(axis) / len(axis)

    def sum_of_deviation_products(self):
        return sum((self.x[i] - self.x_mean) * (y - self.y_mean)
                   for i, y in enumerate(self.y))

    def sum_of_x_deviation_squared(self):
        return sum((x - self.x_mean) ** 2
                   for x in self.x)

    @property
    def slope(self):
        return self.sum_of_deviation_products() / self.sum_of_x_deviation_squared()

    def fit_best_line(self):
        self.b_zero = self.y_mean - (self.slope * self.x_mean)
        print(f"The best line equation is: {self.slope:.2f}x + {self.b_zero:.2f}")


Returns all primes p between m <= p <= n This is my submission for the Prime Generator on SPOJ and it was accepted. Are there any improvements/changes I can make?

  Input:

  The input begins with the number \$t\$ of test cases in a single line
  (\$t \le 10\$). In each of the next t lines there are two numbers \$m\$ and
  \$n\$ (\$1 \le m \le n \le 1000000000\$, \$n-m \le 100000\$) separated
  by a space.

  Output:

  For every test case print all prime numbers \$p\$ such that \$m \le p \le n\$,
  one number per line, test cases separated by an empty line.

  Example:

  Input:

2
1 10
3 5

  Output:

2
3
5
7

3
5

#include <iostream>
#include <cmath>
#include <vector>

using std::vector;
using std::cout;
using std::cin;

bool isPrime(int n) {
    if (n == 1) return false;
    if (n == 2) return true; // invariant
    int root = int(ceil(sqrt(n))); // check up to ceil of square root n
    for (int i = 2; i <= root; ++i){
        if (n % i == 0) return false; // not prime
    }
    return true;
}

int main() {
    int lines;
    int toPush;
    vector<int> inputs;
    // get inputs
    cin >> lines;
    int tempA = 0;
    while (tempA < 2*lines) {
        cin >> toPush;
        inputs.push_back(toPush);
        ++tempA;
    }

    auto i = inputs.begin();
    while (i < inputs.end()){
        int m = *i;
        int n = *(i + 1);
        while (m <= n){
            if (isPrime(m)){
                int value = m;
                cout << value << " ";
            }
            ++m;
        }
        cout << "\n";
        std::advance(i, 2);
    }
}

Here are a few things that may help you improve your program.

Improve your algorithm

Right now, within the isPrime routine, the loop begins at 2 and does a test division of every number from \$2\$ to \$\sqrt{n}\$.  However, we already know that other than 2, all prime numbers are odd.  You can approximately double the speed of this algorithm by writing it like this instead:

bool isPrime(int n) {
    if (n == 1) return false;
    if (n == 2) return true;
    if (n % 2) return false;
    int root = int(ceil(sqrt(n)));
    for (int i = 3; i <= root; i+=2){
        if (n % i == 0) return false;
    }
    return true;
}

Prefer for to while where appropriate

The input section of the code has these lines:

int tempA = 0;
while (tempA < 2*lines) {
    cin >> toPush;
    inputs.push_back(toPush);
    ++tempA;
}

It seems to me that this would be more clear as a for loop:

for(int tempA = 2*lines; tempA; --tempA) {
    std::cin >> toPush;
    inputs.push_back(toPush);
}

The same is true for the main loop in the program.  It could be written this way:

for (auto i = inputs.begin(); i != inputs.end(); ++i) {
    for (int m = *i++, n=*i; m <= n; ++m) {
        if (isPrime(m)){
            std::cout << m << " ";
        }
    }
    std::cout << '\n';
}

Store std::pairs instead of ints

After the first number, the program's input consists of pairs.  It might make more sense to store them in a std::vector<std::pair<int, int>>

The input routine would then look like this:

for(int tempA = 2*lines; tempA; --tempA) {
    std::pair<int, int> toPush;
    std::cin >> toPush.first >> toPush.second;
    inputs.push_back(toPush);
}

The main loop is then considerably simplified by the use of the pair and a "range-for" outer loop:

for (const auto &lim : inputs) {
    for (int m = lim.first; m <= lim.second; ++m) {
        if (isPrime(m)){
            std::cout << m << " ";
        }
    }
    std::cout << '\n';
}

}

Use a better algorithm

The code works as it is, but could be still further improved.  Since the inputs are all read at the beginning, you could choose the largest upper bound and run a sieve of Eratosthenes to derive all primes up to that number.  Then printing for any range would be simply a matter of lookup.


How can I optimize this combination method? I have this method that is working perfectly fine, but it's very slow, and sometimes I have to wait 15 minutes to get a good result, which is ok.  I'm wondering if I can make it faster.

Basically I'm running best fleet simulations, and I pre-calculate the possible ship combinations for specific voyages, but then I have to get the best fleet combinations for multiple voyages.

Obviously I can't use the same captain or crew in different ships because the voyages take place at the same time, and that's why there's some more conditions in each inner loop.

private static Ship[] GetBestFleet3(IList<Voyage> voyages)
{
    var bestRate = 0;
    var worstVariance = 100.0;
    Ship[] fleet = null;
    foreach (var ship0 in voyages[0].Ships)
    {
        foreach (var ship1 in voyages[1].Ships.Where(s1 => !s1.Equals(ship0)))
        {
            foreach (var ship2 in voyages[2].Ships.Where(s2 => !s2.Equals(ship0) && !s2.Equals(ship1)))
            {
                var variance = Statistics.Variance(ship0, ship1, ship2);
                var rate = ship0.Rate + ship1.Rate + ship2.Rate;
                if (rate >= bestRate || rate == bestRate && variance < worstVariance)
                {
                    bestRate = rate;
                    worstVariance = variance;
                    fleet = new[] { ship0, ship1, ship2 };

                    if (ship0.Rate >= 100 && ship1.Rate >= 100 && ship2.Rate >= 100)
                    {
                        return fleet;
                    }
                }
            }
        }
    }
    return fleet;
}

voyages is basically an array of voyages with possible ship combinations for each voyage.

Gist with all the relevant project files: https://gist.github.com/alfaproject/cb68e6fa1c21e1f93bf7

I'd try to go with a different approach. At the moment you're evaluating all the possible combinations, while I think it could be useful to evaluate only the meaningful ones. To do that I'd start with a two phase approach. First you can generate all the valid fleets and then you can sort them using a comparator that takes into account their Range/Variance.

This is the code you can use to generate all the possible fleets:

private static IEnumerable<Fleet> PossibleVoyages(IList<Captain> captains, IList<Crew> crews, int voyagesToPrepare, Fleet fleet)
{
    var possibleVoyages = new List<Fleet>();
    if(voyagesToPrepare == 0)
    {
        possibleVoyages.Add(fleet);
    }
    if(captains.Count < voyagesToPrepare || crews.Count < voyagesToPrepare)
    {
        // We cannot find any valid solution here
        return possibleVoyages;
    }
    else
    {
        foreach(var captain in captains)
        {
            foreach(var crew in crews)
            {
                var newFleet = fleet.Add(CreateShip(captain, crew));
                possibleVoyages.AddRange(
                    PossibleVoyages(
                        FreeCaptains(captains, newFleet),
                        FreeCrews(crews, newFleet),
                        voyagesToPrepare - 1,
                        newFleet));
            }
        }
    }
    return possibleVoyages;
}

I have not implemented the methods FreeCaptains, FreeCrews, and CreateShip but they should be obvious. Fleet act as an immutable collection of ships.

Once you get all the possible voyages you can just sort them using a comparator and Linq sort.

I'm not sure this is going to help much, but it is worth giving it a try.
A possible way to improve it is to keep track of the best fleet we generated so far and to just compare it with every fleet we generate instead of doing all the comparisons at the end. This could possibly prune a lot of solutions once you find a good one and it is definitely worth trying.


Microsoft OA | Longest Substring Without 3 Contiguous Occurrences of Letter For the following question, does my JS Solution seem correct?
I'm looking for some peer review.

Q: Given a string s containing only a and b, find longest substring of s such that s does not contain more than two contiguous occurrences of a and b.

Examples:
"aabbaaaaabb" => "aabbaa"
"aabbaabbaabbaa" => "aabbaabbaabbaa"
function maxLenSubStr(s) {
    let start = 0, end = 0;
    let numOfConsecutiveChars = 0;
    let res = "";
    while (end <= s.length) {
        numOfConsecutiveChars++;
        if (numOfConsecutiveChars > 2 || end == s.length) { // time to end our substring is these two cases
            if (end-start > res.length) res = s.substring(start,end); // if it is not longer, there is no need to store it
            start = end-1; // slide our window so we can look for a new valid substring
            numOfConsecutiveChars--; // now we have a valid substring with two contiguous occurrences instead of three
        }
        if (s[end] !== s[end+1]) numOfConsecutiveChars = 0;
        end++;
    }
    return res;
    // Time Complexity: O(n)
    // Space Complexity: O(1)
}

After some tests, the code does look correct, but the logic takes some time to parse and understand, which is a significant disadvantage; when writing good, maintainable code, readability is usually the most important factor to consider.
If a script is running too slowly for your liking, you can run a performance test to identify the bottleneck(s) and then fix those bottlenecks (possibly with hard-to-read code like this), but before then, better to write easy-to-write, easy-to-understand code first.
If it were me, I'd use a regular expression. It'll have many fewer "moving parts" than going through the string manually and counting up lengths, and REs are quite widely used for general string matching and manipulation across most programming languages; it's something a programmer will probably already be familiar with, so hopefully using them won't present a problem.
The one complication with a regular expression is that when there may be overlapping matches, the code can get somewhat ugly, see here for examples. Here, with a string of abbba, you'd have to consider both abb and bba, where the middle b is contained in both. Rather than deal with overlapping matches, I'll replace all 3+-contiguous-occurrences with 4-contiguous-occurrences beforehand. Then it's trivial to use .split to split along the middle of each 4-contiguous section, and then return the longest string in the resulting array:

const maxLenSubStr = s => s
  // Expand 3 or more contiguous occurrences to 4 contiguous occurrences
  // eg 'abbba' -> 'abbbba'
  .replace(/(.)\1{2,}/g, '$&$1')
  // Split along the middle of each 4-contiguous subsequence
  // eg 'abbbba' -> ['abb', 'bba']
  .split(/(?<=\1(.))(?=\1{2})/)
  .reduce((a, b) => a.length > b.length ? a : b);

console.log(maxLenSubStr("aabbaaaaabb")); //  => "aabbaa"
console.log(maxLenSubStr("aabbaabbaabbaa")); //  => "aabbaabbaabbaa"
console.log(maxLenSubStr("baaa")); //  => "baa"

For the regular expressions, the same technique is being used in both. Capture a character in a group, then backreference that group to match the same character again as many times as needed.

(.)\1{2,} - (.) captures the character, \1{2,} matches it again 2 or more times, then replaces with 4 occurrences of the character.
(?<=\1(.))(?=\1{2}) - (.) captures the character in the lookbehind, preceded by \1, another occurrence of the character. Then (?=\1{2}) looks ahead for 2 more occurrences of the character.

I'd consider this an order of magnitude easier to understand than your original code. It's much easier to see at a glance that it'll carry out the desired logic properly, at least for those with a bit of experience with regular expressions. Whether it's worth replacing your original code with it would depend on you and your team's familiarity with REs.
If you had to do it without lookbehind (which is not supported on out-of-date browsers), then split with another capturing group around the whole left match, or use .match instead, to match characters until encountering 3 in a row:

const maxLenSubStr = s => s
  // Expand 3 or more contiguous occurrences to 4 contiguous occurrences
  // eg 'abbba' -> 'abbbba'
  .replace(/(.)\1{2,}/g, '$&$1')
  // Split along the middle of each 4-contiguous subsequence
  // eg 'abbbba' -> ['abb', 'bba']
  // Lazily match characters until encountering the end of the string
  // or until 3 of the same character are in a row
  .match(/.*?(?:$|(.)\1(?=\1))/g)
  .reduce((a, b) => a.length > b.length ? a : b);

console.log(maxLenSubStr("aabbaaaaabb")); //  => "aabbaa"
console.log(maxLenSubStr("aabbaabbaabbaa")); //  => "aabbaabbaabbaa"
console.log(maxLenSubStr("baaa")); //  => "baa"
console.log(maxLenSubStr("aaab")); //  => "aab"

If you were to keep using your original code, I'd suggest:

Use strict comparison === instead of sloppy comparison == (which has weird coercion rules a reader of the code shouldn't have to worry about)

Use spaces between non-unary operators and operands, for readability, eg end - start > res.length instead of end-start > res.length

Rather than numOfConsecutiveChars--;, since the new consecutive count will always be 2, better to assign 2 to it directly: numOfConsecutiveChars = 2

Increment numOfConsecutiveChars only when a subsequent consecutive character is found, and not unconditionally. (Your current logic works, it's just confusing)


Animation for collapsing/expanding a navigation bar I wrote the following to create an animation which collapses a navigation bar, load new content for it and expands it again. Meanwhile, the rest of the page content fades out, loads and fades in again.

It works and looks good, and is even timed well. But my code is one hell of a mess. How could I have done better?

$(function() {
var newHash      = "",
    $NavmainContent = $("#contentNavigationWrapper"),
    $mainContent = $(".contentWrapper"),
    $first       = $('#currentSection'),
    $second      = $('#project-tools'),
    $third       = $('#project-pagetools'),
    $fourth      = $('#cp'),
    $pageWrap    = $("#page-wrap"),
    baseHeight   = 0,
    $el;

$pageWrap.height($pageWrap.height());
baseHeight = $pageWrap.height() - $NavmainContent.height();

$("nav").delegate("a", "click", function() {
    _link = $(this).attr("href");
    history.pushState(null, null, _link);
    loadContent(_link);
    return false;
});

function loadContent(href){

    $mainContent
        .find("#guts")
        .fadeOut(1000, function() {
            $mainContent.hide().load(href + " #guts", function() {
                $mainContent.fadeIn(1000);

            });
        });

    $NavmainContent
        .find('#inner-cn')
        .find("#project-pagetools,#project-tools,#cp,#currentSection")
        .animate({
            'width': '0'
        },800);

    setTimeout(function(){
        $first.load(href + " #currentSection span");
        $second.load(href + " #project-tools li");
        $third.load(href + " #project-pagetools li");
        $fourth.load(href + " #cp span");
    },900);

    setTimeout(function(){

        var l = $("#project-tools li").length ;

        l = 100 / l ;
        l = l + '%';
        $("#project-tools li").css({
            'width': l
        });

        $NavmainContent
            .find("#project-pagetools,#project-tools")
            .animate({
                'width': '15%'
            },800);

        $NavmainContent
            .find("#currentSection")
            .animate({
                'width': '10%'
            },800);

        $NavmainContent
            .find("#cp")
            .animate({
                'width': '55%'
            },800);

    },1100);

}

});

Here are some quite general pointers on your code. If you put up a fiddle with everything working, I might be able to play around and re-write this for you. But anyways, I've included my comments directly into the code to provide some context:

$(function() {//Is this what wraps your entire code, or just this part of it?
    var newHash      = "",
        $NavmainContent = $("#contentNavigationWrapper"),
        $mainContent = $(".contentWrapper"),
        $first       = $('#currentSection'),
        $second      = $('#project-tools'),
        $third       = $('#project-pagetools'),
        $fourth      = $('#cp'),
        $pageWrap    = $("#page-wrap"),
        baseHeight   = 0,
        $el; //You define "el" but don't use it anywhere.

    $pageWrap.height($pageWrap.height()); //Da fuck? I don't get this lol. You set the height of the element to its own height?
    baseHeight = $pageWrap.height() - $NavmainContent.height(); //I would just include a pageWrapHeight variable since you use often.

    $("nav").delegate("a", "click", function() { //.delegate is deprecated since 1.7, use .on() instead: $("nav").on("click", "a", callback(){});
        _link = $(this).attr("href");
        history.pushState(null, null, _link);
        loadContent(_link);
        return false; //Here you can just e.preventDefault(); instead. Don't forget to pass the "e" for event into the callback.
    });

    function loadContent(href){

        $mainContent
            .find("#guts")
            .fadeOut(1000, function() {
                $mainContent.hide().load(href + " #guts", function() {
                    $mainContent.fadeIn(1000);
                });
            });

        $NavmainContent
            .find('#inner-cn')
            .find("#project-pagetools,#project-tools,#cp,#currentSection")
            .animate({
                'width': '0'
            },800);

            //The method ".load()" calls the method ".ajax()", passing in the stuff you put there.
            //Why not save a few function calls and just use the .ajax() method directly?

            //Instead of using this timeout, you should check out ajax a little more.
            //You'll find that it has a nice little thing called Deffered already built into the ajax method.
            //Also jQuery has a queue which you can manipulate the order of stuff that happens etc., animation in particular.
        setTimeout(function(){
            $first.load(href + " #currentSection span");
            $second.load(href + " #project-tools li");
            $third.load(href + " #project-pagetools li");
            $fourth.load(href + " #cp span");
        },900);

        //Same thing here.
        //You could do this all with a deffered promise.
        //Also worth checking out is Pub/Sub
        //Basically when set up, you can publish, "Hey I'm done doing this stuff" to another part of your JS that is "subscribed" to that,
        //and continue with your other stuff only when that gets published.
        setTimeout(function(){

            var l = $("#project-tools li").length; //I don't personally like using single character variable names.
            //That can be a nightmare to come back to in the future if you have to make changes or include new functionality.
            //But then again that's just me. Don't shoot yourself over that.

            l = 100 / l ;
            l = l + '%';
            $("#project-tools li").css({
                'width': l
            });

            $NavmainContent
                .find("#project-pagetools,#project-tools")
                .animate({
                    'width': '15%'
                },800);

            $NavmainContent
                .find("#currentSection")
                .animate({
                    'width': '10%'
                },800);

            $NavmainContent
                .find("#cp")
                .animate({
                    'width': '55%'
                },800);
        },1100);
    }
});


fasm convert hex byte to binary string This fasm x64 (Linux) code seems very crude and repetitive, but it gets the job done. How could I perform this task in a more idiomatic manner?

format ELF64 executable 3

segment readable executable

entry $

prompt_user:
mov edx,prompt_len
lea rsi,[prompt] ;<------ String
mov edi,1       ; STDOUT
mov eax,1       ; sys_write
syscall

get_user_input:
mov rax, 0 ; sys_read
mov rdi, 0 ; STDIN
lea rsi, [bit_string] ; <----- defined memory location
mov rdx, 2 ; <---- # of chars to read
syscall

;DEBUG CODE:
;lea rax, [bit_string]
;mov [rax], byte 'A'
;mov [rax+1], byte 'B'
lea rax, [bit_string]
cmp [rax], byte '0'
je load_0
cmp [rax], byte '1'
je load_1
cmp [rax], byte '2'
je load_2
cmp [rax], byte '3'
je load_3
cmp [rax], byte '4'
je load_4
cmp [rax], byte '5'
je load_5
cmp [rax], byte '6'
je load_6
cmp [rax], byte '7'
je load_7
cmp [rax], byte '8'
je load_8
cmp [rax], byte '9'
je load_9
cmp [rax], byte 'A'
je load_A
cmp [rax], byte 'B'
je load_B
cmp [rax], byte 'C'
je load_C
cmp [rax], byte 'D'
je load_D
cmp [rax], byte 'E'
je load_E
cmp [rax], byte 'F'
je load_F
load_0:
lea rsi, [n0]
jmp print_nibble
load_1:
lea rsi, [n1]
jmp print_nibble
load_2:
lea rsi, [n2]
jmp print_nibble
load_3:
lea rsi, [n3]
jmp print_nibble
load_4:
lea rsi, [n4]
jmp print_nibble
load_5:
lea rsi, [n5]
jmp print_nibble
load_6:
lea rsi, [n6]
jmp print_nibble
load_7:
lea rsi, [n7]
jmp print_nibble
load_8:
lea rsi, [n8]
jmp print_nibble
load_9:
lea rsi, [n9]
jmp print_nibble
load_A:
lea rsi, [nA]
jmp print_nibble
load_B:
lea rsi, [nB]
jmp print_nibble
load_C:
lea rsi, [nC]
jmp print_nibble
load_D:
lea rsi, [nD]
jmp print_nibble
load_E:
lea rsi, [nE]
jmp print_nibble
load_F:
lea rsi, [nF]
jmp print_nibble

nibble_two:
lea rax, [bit_string]
cmp [rax+1], byte '0'
je load_0
cmp [rax+1], byte '1'
je load_1
cmp [rax+1], byte '2'
je load_2
cmp [rax+1], byte '3'
je load_3
cmp [rax+1], byte '4'
je load_4
cmp [rax+1], byte '5'
je load_5
cmp [rax+1], byte '6'
je load_6
cmp [rax+1], byte '7'
je load_7
cmp [rax+1], byte '8'
je load_8
cmp [rax+1], byte '9'
je load_9
cmp [rax+1], byte 'A'
je load_A
cmp [rax+1], byte 'B'
je load_B
cmp [rax+1], byte 'C'
je load_C
cmp [rax+1], byte 'D'
je load_D
cmp [rax+1], byte 'E'
je load_E
cmp [rax+1], byte 'F'
je load_F

print_intro:
mov edx,intro_len
lea rsi,[intro]
mov edi,1       ; STDOUT
mov eax,1       ; sys_write
syscall
ret

print_nibble:
cmp [position], 0 ; If were on first iter, then print intro
jne skip_intro
push rsi
call print_intro
pop rsi

skip_intro:
mov rax, 1
cmp al, [position]
jl go_out
mov edx, 4

mov edi,1       ; STDOUT
mov eax,1       ; sys_write
syscall
add [position], 1
jmp nibble_two

go_out:
mov edx,1

lea rsi,[nl]
mov edi,1       ; STDOUT
mov eax,1       ; sys_write
syscall

xor edi,edi     ; exit code 0
mov eax,60      ; sys_exit
syscall

segment readable writeable
prompt db 'Enter a byte in the format: F6', 0xA
prompt_len = $ - prompt
nl db 0xA
intro db 'In binary: ', 0
intro_len = $ - intro
bit_string rb 2
position db 0
n0 db '0000', 0
n1 db '0001', 0
n2 db '0010', 0
n3 db '0011', 0
n4 db '0100', 0
n5 db '0101', 0
n6 db '0110', 0
n7 db '0111', 0
n8 db '1000', 0
n9 db '1001', 0
nA db '1010', 0
nB db '1011', 0
nC db '1100', 0
nD db '1101', 0
nE db '1110', 0
nF db '1111', 0

In a high-level language, I would write it as:

def put_nibble(buf: array[byte], n: int):
    buf[0] = '0' + ((n shr 3) bitand 1)
    buf[1] = '0' + ((n shr 2) bitand 1)
    buf[2] = '0' + ((n shr 1) bitand 1)
    buf[3] = '0' + ((n shr 0) bitand 1)

def char_to_nibble(ch: byte) -> int:
    if '0' <= ch <= '9': return ch
    ch = ch bitor 0x20  # convert to lowercase
    if 'a' <= ch <= 'z': return ch - 'a' + 10
    error

def byte_to_binary(b: str):
    buf = byte[8]  # reserve memory on the stack
    hi = char_to_nibble(b[0])
    put_nibble(buf, hi)
    lo = char_to_nibble(b[1])
    put_nibble(but + 4, lo)
    sys_write(1, buf, 8)

This way you avoid comparing each possible digit on its own.

The above code also reduces memory access to the memory on the stack, since it doesn't need any external strings for the bit patterns.

Sure, the put_nibble code looks a bit repetitive, but you can merge all the '0' + … together into a single addition:

add dword [buf], 0x30303030  ; 0x30 == '0'

You could also compute the buffer for a whole nibble in a register and then write it to memory in a single instruction:

; input: al = the nibble to print
; output: ebx = 4 bytes ASCII buffer containing the 4 binary digits
xor ebx, ebx

rcr al, 1
adc ebx, '0'
rol ebx, 8

rcr al, 1
adc ebx, '0'
rol ebx, 8

rcr al, 1
adc ebx, '0'
rol ebx, 8

rcr al, 1
adc ebx, '0'
rol ebx, 8

mov [buf], ebx

Once you have translated the above high-level code into assembler, you probably want to optimize the code a bit. Or just use a compiler to do the heavy work for you. Write your code in C or in Go, assemble it into machine code and look at the result. Some keywords for searching:

go tool objdump
gcc -S binary.c -O2

By the way, you should not place the string literals for the prompt and the bit patterns in a writeable segment since they are not intended to be overwritten by any part of your program.