Reading image from jar I am using this static method in my class IconManager to read an image from jar file. imagepath is a relative path to the image.

This code works perfect. But I was told it is error-prone, because does not handle Exceptions correctly.

private static BufferedImage readBufferedImage (String imagePath) {
    try {
        InputStream is = IconManager.class.getClassLoader().getResourceAsStream(imagePath);
        BufferedImage bimage = ImageIO.read(is);
        is.close();
        return bimage;
    } catch (Exception e) {
        return null;
    }
}

How would you refactor the code?

Don't catch (or throw) Exception.
Try not to return null. If you cannot return a normal value you want one of the following to happen:

Program quits with an error. (Your current program seems to do this)
You expect calling site to take some action and try to recover and continue execution
You do not want to hassle the caller of the method and try to continue execution with a sensible default value.

If you want program to quit with an error. Wrap the exception in a runtime exception with a meaningful (to the developer, not user) message. That way in your logs you will see what went wrong and where. If you return a null what you probably get is a nondescript NullPointerException down the line.

If you want the caller of the method to check if something went wrong, you want to throw a checked exception. Null checks always get ignored and your program crashes and burns in inopportune moments and users think, rightly, the program is of low quality. If you do not have and cannot create a suitable Exception, you can propagate the exception.

Third option is usually ignored but sometimes continuing with some default value is the most logical thing to do. Think what browsers do if they cannot download an image, which basically is an IOException. They show a white rectangle with a red cross on it. A web page with some of the images missing is more useful than a page full of stack trace. And note that a white rectangle with a cross behaves like a normal image in terms of layout, hence do not break the page layout. This is the essence of the Null Object Pattern.
(Updated as per @Sulthan) You should call the InputStream.close() in a finally block. Because if ImageIO.read throws an exception you never reach the close() statement.

A few notes about the example code below:

Resource leaks may occur if InputStream.close throws. There isn't much you can do about it.
Wrapping example and the ImageReadException do not pull their weights as they are. they are here for demonstration.
Method names are shortened to fit screen.

Here is the basic case:

private static BufferedImage readPropagate(String imagePath) throws IOException {
    InputStream is = null;
    try {
        is = IconManager.class.getClassLoader().getResourceAsStream(imagePath);
        // an example of how null checks can easily be forgotten
        if (is == null) {
            throw new FileNotFoundException("Resource not found: " + imagePath);
        }
        return ImageIO.read(is);
    } finally {
        if (is != null) {
            is.close();
        }
    }
}

Here are some examples of what can be done when the above example is not sufficient:

public static class ImageReadException extends Exception {
    private static final long serialVersionUID = 1L;

    public ImageReadException(String message, Throwable cause) {
        super(message, cause);
    }

    public ImageReadException(String message) {
        super(message);
    }
}

private static BufferedImage readWrap(String imagePath) throws ImageReadException {
    try {
        return readPropagate(imagePath);
    } catch (IOException e) {

        // log with your normal logger
        logException(e, "An error occured while loading image " + imagePath);

        // Message should be something meaningful to the calling site
        throw new ImageReadException("An error occured while loading image", e);
    }
}

private static BufferedImage readWithDefault(String imagePath) {
    try {
        return readPropagate(imagePath);
    } catch (IOException e) {

        // log with your normal logger
        logException(e, "An error occured while loading image " + imagePath);

        //default image may be passed as a parameter instead
        return IconManager.DEFAULT_IMAGE;
    }
}

// YOU SHOULD NOT DO THIS IN LIBRARY CODE
// The decision of whether to crash the program belongs to the top level application
private static BufferedImage readThrowSilently(String imagePath) {
    try {
        return readPropagate(imagePath);
    } catch (IOException e) {
        // you can do this, e.g., while loading a desktop application
        // you discover a vital resource could not be loaded
        // continuing execution does not make sense
        throw new RuntimeException("An error occured while loading image " + imagePath, e);
    }
}

private static void logException(IOException e, String message) {
    Logger.getLogger(IconManager.class.getName()).log(Level.SEVERE, message, e);
}


JavaScript / jQuery selector value function I have the following function which opens a modal and puts some value into a div. It works but the function looks ugly, I hope for improvements (maybe more dynamic).

submitDelete: function() {

    // projects
    $('button[name="removeProj"]').on('click', function(e){
        var $form      = $(this).closest('form'); // closest parent form
        var $id        = $(this).data('id');
        var $title     = $(this).data('title');
        var $createdAt = $(this).data('created-at');
        var $updatedAt = $(this).data('updated-at');
        e.preventDefault();
        if($id != '') {
            $("#prodId").empty().append($id);
        } else {
            $("#prodId").empty().append('-');
        }
        if($title != '') {
            $("#prodTitle").empty().append($title);
        } else {
            $("#prodTitle").empty().append('-');
        }
        if($createdAt != '') {
            $("#prodCreatedAt").empty().append($createdAt);
        } else {
            $("#prodCreatedAt").empty().append('-');
        }
        if($updatedAt != '') {
            $("#prodUpdatedAt").empty().append($updatedAt);
        } else {
            $("#prodUpdatedAt").empty().append('-');
        }
        $('#confirm').modal({ backdrop: 'static', keyboard: false })
        $('#delete').click(function() {
            $form.trigger('submit'); // submit the form
        });
    });

In JavaScript you can do this:

var example = someValue || 'fallbackValue';

if someValue does not have a value, example=='fallbackValue', otherwise it'll be whatever value someValue has; You can integrate this in your code.

// You can now replace this:
if($id != '') {
    $("#prodId").empty().append($id);
} else {
    $("#prodId").empty().append('-');
}
// With this:
$("#prodId").empty().append( $(this).data('id') || '-')
// Or just set it directly:
$("#prodId").val( $(this).data('id') || '-')

Resulting in:

submitDelete: function() {

    // projects
    $('button[name="removeProj"]').on('click', function(e){
        e.preventDefault();
        var $form      = $(this).closest('form'); // closest parent form

        $("#prodId").val(        $(this).data('id') || '-');
        $("#prodTitle").val(     $(this).data('title') || '-');
        $("#prodCreatedAt").val( $(this).data('created-at') || '-');
        $("#prodUpdatedAt").val( $(this).data('updated-at') || '-');

        $('#confirm').modal({ backdrop: 'static', keyboard: false })
        $('#delete').click(function() {
            $form.trigger('submit'); // submit the form
        });
    });
}

*Instead of .val() you can use .text() or .html() to set the values of other types of elements like divs.


Tic-Tac-Toe game in Python 3 I have written a code for a tic-tac-toe game and it works well. But, I don't know whether it is an efficient one. Any help on how to optimize it?

def restart():

    num=[0 for i in range(9)]
    board=[" " for i in range(9)]
    a=2
    start(board,a,num)

def wincheck(t):

    for i in range(8):
        if t[win[i][0]]==t[win[i][1]] and t[win[i][0]]==t[win[i][2]] and
                            t[win[i][1]]==t[win[i][2]] and t[win[i][0]]!=" ":
            if t[win[i][0]]=="X":
                print("Player 1 wins")
            else:
                print("Player 2 wins")
            playa = input("Do you want to play again? Y/N")
            if playa.lower() == "y":
                restart()
            else:
                exit(0)
def display_board(t,a,num):

    print( " "*3+'|'+" "*3+'|'+" "*3)
    print("{0:^3}|{1:^3}|{2:^3}".format(t[6],t[7],t[8]))
    print( " "*3+'|'+" "*3+'|'+" "*3)
    print("-"*11)
    print( " "*3+'|'+" "*3+'|'+" "*3)
    print("{0:^3}|{1:^3}|{2:^3}".format(t[3],t[4],t[5]))
    print( " "*3+'|'+" "*3+'|'+" "*3)
    print("-"*11)
    print( " "*3+'|'+" "*3+'|'+" "*3)
    print("{0:^3}|{1:^3}|{2:^3}".format(t[0],t[1],t[2]))
    print( " "*3+'|'+" "*3+'|'+" "*3)
    wincheck(t)
    if 0 not in num:
        print("Draw")
        restart()
    start(t,a,num)

def start(board,a,num):

    if a%2==0:
       p1n=int(input("Player 1, Enter a number from 1 to 9"))
       if p1n not in num:
           num[p1n-1]=p1n
           print(num)
       else:
           print("Please enter in a free space")
           start(board,a,num)
       board[p1n-1]="X"
       a=a+1
       display_board(board,a,num)
    else:
       p2n=int(input("Player 2, Enter a number from 1 to 9"))
       if p2n not in num:
           num[p2n-1]=p2n
           print(num)
       else:
           print("Please enter in a free space")
           start(board,a,num)
       board[p2n-1]="O"
       a=a+1
       display_board(board,a,num)

a=2

board=[" " for i in range(9)]

num=[0 for i in range(9)]

win = [[0,1,2],[3,4,5],[6,7,8],[0,4,8],[6,4,2],[6,3,0],[7,4,1],[8,5,2]]

start(board,a,num)

Any help on how to optimize it?

Optimise readability, starting with following the Style Guide for Python Code.
(And yes, this is a useful step in optimising efficiency, too)

(The below reads an ill-tempered diatribe more than a code review, the sole exculpation being the explicit question in the title. (I should look for a well-receives SE resource bound to exist.))

How to write efficient source code for …?
Given a specification that looks ready for implementation,
- write tests shedding a light on correctness.
- fix(!) a limit for the effort to improve efficiency
- define a measure for efficient
(-if applicable, define relevant input data)
- automate measuring efficiency - memory usage is harder to assess than time used
 (use available tools - timeit, for starters)
 (It saves a lot of trouble to include (basic) functional tests here)
 (Don't believe sub-second measurements.
  Measure work completed in a fixed amount of time, not time used for a fixed amount of work.)
- use your first working version to establish a base line
- fix(!) what performance is good enough
- pick alternative implementations, jot down your guesses re. efficiency, measure
- time allowing (see limit above), experiment with improvements


Unit test code for a private median-of-three function, using reflection A Junit test class is used to test the class LibraryCounter. Reflection was used to test the private methods. This made the code much more complicated.

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.junit.Test;
import static org.junit.Assert.*;

public class LibraryCounterTest {

 public LibraryCounterTest() { }

 @Test
 public void medianEasyTest() {
     System.out.println("median");
     int[] sample = {7,8,9};
     Class[] args = new Class[3];
     args[0] = int[].class;
     args[1] = Integer.TYPE;
     args[2] = Integer.TYPE;

     try {
           Method m = LibraryCounter.class.getDeclaredMethod("median", args);
          m.setAccessible(true);
          try {
              Object o = m.invoke(null, sample, 0, sample.length-1);
              int result = (int)o;
              assertEquals(8, result);
          } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException ex) {
             Logger.getLogger(LibraryCounterTest.class.getName()).log(Level.SEVERE, null, ex);
        }

    } catch (NoSuchMethodException | SecurityException ex) {
         Logger.getLogger(LibraryCounterTest.class.getName()).log(Level.SEVERE, null, ex);
    }

     assertEquals(7, sample[0]);
     assertEquals(8, sample[1]);
     assertEquals(9, sample[2]);
 }
}

This is just the first and simplest test.  The method being tested has the following signature private static int median(int[], int, int) If the method was public then everything inside the outermost try could be condensed to one line.

Here is the method being tested from the LibraryCounter class:

/*
*Sorts first, center and last element, swaps the new center element with the one before the new last and returns its value.
*/
private static int median(int[] sample, int start, int end) {
    if(sample.length < 3) {
        throw new IllegalArgumentException("arrays of length three or greater");
    }

    int center = start + ((end-start)/2);

    if(sample[start] > sample[end])
       swap(sample, start, end);

    if(sample[start] > sample[center])
       swap(sample, start, center);

    if(sample[center] > sample[end])
       swap(sample, center, end);

int secondLast = end - 1

    swap(sample, center, secondLast );

    return sample[secondLast];

}

//swaps two elements in array given their positions
private static void swap(int[] sample, int x, int y) {
    int temp = sample[x];
    sample[x] = sample[y];
    sample[y] = temp;
}

As I've said in my comments, I'm not a huge fan of testing private methods.  It couples the tests very closely to the implementation of your class which makes even simple refactorings much harder than they need to be.  That said, lets have a look at your test.

Naming

medianEasyTest tells me nothing really about what the test is expecting.  median is the name of the method you're calling.  EasyTest tells me nothing about what you're passing, or what you're expecting back.  This doesn't make the intention of the test easy to read.

Exceptions

You're catching exceptions in your test and more than that, whilst you're logging them, you're swallowing the exceptions.  I really don't like that.  If your code throws an exception, you should let the exception escape the test and fail it.  Having the test fail on assertEquals(7, sample[0]); when it's actually failed because it threw an IllegalAccessException is just confusing.

Setup

If you're going to be running multiple tests against the same private methods, then it makes sense to do a lot of the prep work either in a method called explicitly, or in a Before method on the test class (notice that I'm not catching any exceptions.  If they happen, because your class structure changes you want it to fail:

@Before
public void setupPrivateCalls() {
   Class[] args = new Class[3];
   args[0] = int[].class;
   args[1] = Integer.TYPE;
   args[2] = Integer.TYPE;

   medianMethod = LibraryCounter.class.getDeclaredMethod("median", args);
   medianMethod.setAccessible(true);
}

Method medianMethod;

Double checking

You're checking that sample hasn't been updated at the end of your test.  Do you really need to do that?

Putting it together

Putting all of that together, you end up with an actual test method that looks more like this:

public void testMedianIs8From789() {
     System.out.println("median");
     int[] sample = {7,8,9};

     int result  = (int)medianMethod.invoke(null, sample, 0, sample.length-1);
     assertEquals(8, result);
 }

Exceptions from privates

This is probably subjective, but I don't like the way your private method is throwing an exception:

private static int median(int[] sample, int start, int end) {
    if(sample.length < 3) {
        throw new IllegalArgumentException("arrays of length three or greater");
    }

If your private throws that exception, it's because something under your classes control (one of it's public methods) has failed to supply the right data.  Validation should be performed at the public interface to the class, you shouldn't need to perform it again in your privates.


Longest common substring using dynamic programming I've written a short python script that attempts to solve the problem of finding the longest common substring using the dynamic programming technique. It is meant to be generalised so I could plug in any number of strings and it would find the longest common substring.

def longest_common_substring(*strings):
    table = defaultdict(int)
    for pos in product(*(range(len(s)) for s in strings)):
        same = len(set(s[i] for s, i in zip(strings, pos))) is 1
        table[pos] = table[tuple(map(lambda n: n - 1, pos))] + 1 if same else 0
    return max(table.items(), key=operator.itemgetter(1))

This works fine for a small number of short strings, but the space and time complexity absolutely blows up with longer strings.

I got this from wikipedia, and since this approach is clearly terrible for multiple longer strings (or maybe my implementation is just bad!?), I am wondering what I could do to improve it? Wikipedia also metions a generalised suffix trees... I am not familiar with them at all so would that be a better approach?

Also, if its my implementation, I'd love to know what's wrong and what I could do better in terms of space complexity.

Sorry I've not examined too closely your code to make a comment, but just considering the problem as stated and the fact you are using Py3, I would probably solve it with itertools.accumulate, e.g.:

>>> import itertools as it
>>> import operator as op
>>> ss = ["thisishello", "dfdsishdllo", "ashsisdsdsf"]
>>> i, l = max(enumerate(it.accumulate(it.chain([0], zip(*ss)),
...                      lambda x, y: (x+1)*(len(set(y)) == 1))), key=op.itemgetter(1))
>>> i, l, ss[0][i-l:i]
(6, 3, 'sis')

Should work well for an arbitrary number of strings as it uses generators and doesn't create any intermediate data structures.
It does use the fact that a False equates to 0 with len(set(y)) == 1 but if that is uncomfortable you could simply replace with 1 if len(set(y)) == 1 else 0.

Note: I still wish that itertools.accumulate had an initial value argument much like functools.reduce has, would avoid the need of chaining the initial value to the iterable.


Enum wrapped in a struct I was thinking about how I was going to use some enums I made, how I was going to make sure they had valid values and how to call the class which would contain the necessary extensions methods. And then I thought: why not wrap the enum in a struct. I wrote up a little example code and it looks like this (ignore the fact that it's a very common day enum, I needed something as an example):

public struct Day
{
    private enum DayE
    {
        Monday = 1,
        Tuesday,
        Wednesday,
        Thursday,
        Friday
    }

    private readonly DayE value;

    public static Day Monday { get { return new Day( DayE.Monday ); } }
    public static Day Tuesday { get { return new Day( DayE.Tuesday ); } }
    public static Day Wednesday { get { return new Day( DayE.Wednesday ); } }
    public static Day Thursday { get { return new Day( DayE.Thursday ); } }
    public static Day Friday { get { return new Day( DayE.Friday ); } }

    private Day( DayE value )
    {
        this.value = value;
    }

    public override string ToString()
    {
        switch( value )
        {
            case DayE.Monday:
                return "mo";
            case DayE.Tuesday:
                return "tu";
            case DayE.Wednesday:
                return "we";
            case DayE.Thursday:
                return "th";
            case DayE.Friday:
                return "fr";
            default:
                throw new InvalidOperationException( "Can't convert 0 value" );
        }
    }
}

Some advantages of this approach I could think of:

Allows you to limit the possible values, only the ones you construct using the private constructor can be made. (Of course, the 0 value can always be constructed using default(Day) or new Day().) This means you only have to test if the value is not 0 to make sure it's valid. If 0 is a valid value, you don't have to test for validity.
No need for an helper class for the extension methods, you can just put them in the struct. Also, you can override the ToString method, as can be seen in the example.
In a similar fashion, static methods are now possible.
I think it should be equally fast to just using the enum, since the compiler will probably take away most of the wrapper. Unless, of course, you add functionality, like validity checks, to methods.

Disadvantages:

Need to create public static properties for all enum values. Although this does allow you to create extra values from combinations of other values, that you don't want to put in the enum itself (useful for tweaking the ToString behavior of a enum with Flags attribute).
Need to define methods for all common enum functionality (for each wrapper you make), like HasFlag and ==, !=, & and | operators. However, this gives you control over which of these methods/operators are available and it allows for validity checks inside them. It's possible to create one or more VS code snippets for this.
Visual Studio's auto complete will be a little less happy with this. With enums it usually immediately auto selects the enum type name when you type e.g. Day x =. With this you'll first have to (partially) type that.
To use this in a switch statement, you need to make the internal value and enum public, because structs don't work in a switch.

What do you think of this construction? Can you name any more disadvantages? Would you recommend me not to use this?

Edit: found a new disadvantage, see last point above.

To use static instances of value types instead of plain enum isn't a strange thing. I can't discuss its merits because each case is different (for sure it's slightly slower but you will hardly notice that) but you can see it often (also in .NET Framework code.) Use it cum grano salis because IMO there aren't many use cases unless you want to emulate more powerful Java's enum.

What's wrong, in my opinion, is that you're mixing too many responsibilities inside the same class. Note that you also have code for representation (abbreviated enum value) which usually doesn't belong to domain.

Second point is to wrap the enum. If you use this pattern then you don't need an enum any more, just drop it and integer value (day of week) will have its own clear name.

Third point is initialization, you don't need a property, because Day is an immutable value type your code may be simplified to a static readonly field.

Let's imagine to declare your struct like this:

public struct Day
{
    public static readonly Day Monday = new Day(1);
    public static readonly Day Tuesday = new Day(2);

    private Day(int dayOfWeek)
    {
        _dayOfWeek = dayOfWeek;
    }

    private readonly int _dayOfWeek;
}

Now we have to make it easy to use during our debugging sessions (and integer value isn't so friendly). We have two options here, adding another field with long name (which eventually might hold also the short one) like this:

public static readonly Day Monday = new Day(nameof(Monday), 1);

Note that the main point is to keep everything in one single point. To add another day (!), for example Sunday, you need to change only one point in your code.

If you need it only for debugging then I'd avoid to override ToString() and I'd go with DebuggerDisplayAttribute but it's a matter of preference. ToString() is sometimes the mandatory choice for type conversion in many poor-designed libraries and I'd avoid to use it as debugging aid when I have better options.

[DebuggerDisplay("{_name}")]
public struct Day { /* */ }

Second option is to write a type ad-hoc for debugging purposes without polluting your value type with unused and heavy strings. Let's introduce DebuggerTypeProxyAttribute:

[DebuggerTypeProxy(typeof(DayDebugView))]
public struct Day { /* */ }

In DayDebugView you can put everything you need for debugging:

internal sealed class DayDebugView
{
    public DayDebugView(Day day)
    {
        _day;
    }

    public string Name
    {
        get
        {
            // Pseudo-code with some Reflection Magic
            // To get the static field name...
            return FindStaticFieldByValue(typeof(Day), _day)?.Name;
        }
    }

    private readonly Day _day;
}

Now you're done (in one way or another) with your debugging aid. If you need a presentation value then get rid of those hard-coded strings (because they're not localization friendly). Note that you might want, it depends on your usage pattern, use/implement IFormatProvider (in a separate class), implement IFormattable, implement a TypeConverter (in a separate class), implement IConvertible or use ValueConverterAttribute and implement IValueConverter. Which one is better/required depends on your UI requirements. For UI ToString() is just an abused handy method...

Add to all the above also code for comparison (at least overriding Equals() and implementing IEquatable<T> but probably also == and !=), eventually type conversions (to/from integers?), parsing to/from strings, serialization and...do you also need flags? Then few test to exercise you did everything correctly and you will ask yourself why you're not using a simple enum (with few lines of validation when it's an input.) Yes, sometimes you will need this structure but less often than you might imagine.


SQL Server Split function I know there are some answers about a user function to split chars and I have one already running (found one years ago in the net and modified it to my own needs).

Since I use this function very often, I'd like to ask the best SQL professionals (which are probably here) to review this function for performance. Is the function ok or is there a faster or better way to do this now? Minimum requirements are SQL Server 2005 to work, but if there are better ways from 2008 I can roll out two versions (most usage of the DB is 2008, only a few 2005 left).

CREATE FUNCTION [dbo].[fn_Split](@text nvarchar(4000), @delimiter char(1) = ',')

RETURNS @Strings TABLE
(
position int IDENTITY PRIMARY KEY,
value nvarchar(4000)
)

AS
BEGIN

DECLARE @index int
SET @index = -1

SET @text = RTRIM(LTRIM(@text))

WHILE (LEN(@text) > 0)
BEGIN
  SET @index = CHARINDEX(@delimiter , @text)
  IF (@index = 0) AND (LEN(@text) > 0)
  BEGIN
    INSERT INTO @Strings VALUES (@text)
    BREAK
    END
    IF (@index > 1)
    BEGIN
    INSERT INTO @Strings VALUES (LEFT(@text, @index - 1))
    SET @text = RIGHT(@text, (LEN(@text) - @index))
  END
  ELSE
    SET @text = RIGHT(@text, (LEN(@text) - @index))
  END
  RETURN
END

try this:

Create function dbo.SplitString(@inputStr varchar(1000),@del varchar(5))
RETURNS @table TABLE(col varchar(100))
As
BEGIN

DECLARE @t table(col1 varchar(100))
INSERT INTO @t
select @inputStr

if CHARINDEX(@del,@inputStr,1) > 0
BEGIN
    ;WITH CTE1 as (
    select ltrim(rtrim(LEFT(col1,CHARINDEX(@del,col1,1)-1))) as col,RIGHT(col1,LEN(col1)-CHARINDEX(@del,col1,1)) as rem from @t
    union all
    select ltrim(rtrim(LEFT(rem,CHARINDEX(@del,rem,1)-1))) as col,RIGHT(rem,LEN(rem)-CHARINDEX(@del,rem,1))
    from CTE1 c
    where CHARINDEX(@del,rem,1)>0
    )

        INSERT INTO @table
        select col from CTE1
        union all
        select rem from CTE1 where CHARINDEX(@del,rem,1)=0
    END
ELSE
BEGIN
    INSERT INTO @table
    select col1 from @t
END

RETURN

END


Is there a beter way to check input besides a switch case? So I am running a Switch case to check if a user has filled in all the necessary fields within a form, and if not I prompt them to do so. I was wondering if there was a better way to do this, or if a switch case like this would suffice. General suggestions/naming improvements would also be appreciated.

This runs inside the unity engine, on unity C#

void CheckEmptyValues()
{
    /* Step wise checks certain Necessary values, if things are not assigned it breaks the operation
     * And prompts the user to fill in the appropriate slot
     */
    int caseSwitch = 0;
    switch (caseSwitch)
    {
        case 0:
            if (GalleryName == null || GalleryName == "Default")
            {
                Debug.Log("No Gallery name assigned, please give the gallery a proper name");
                break;
            }
            else goto case 1;
        case 1:
            //This is a rectangle
            if (gallerySize.x == 0 || gallerySize.y == 0 || gallerySize.width == 0 || gallerySize.height == 0)
            {
                Debug.Log("No gallery Sizes assigned, please set some sizes before creating a gallery");
                break;
            }
            else goto case 2;

        case 2:
            if (buttonPrev == null || buttonNext == null)
            {
                Debug.Log(" Please make sure you assigned all textures before building, or you might end up with invisible buttons");
                break;
            }
            else goto case 3;

        case 3:
            //Location they want to get their images from (individual pick, or by path loading)
            if (!indvImages && !imgByPath)
            {
                Debug.Log("Please select a place to find images.");
                break;
            }
            else goto case 4;

        case 4:
            //Enable Bullet Buttons for the gallery
            if (enbBB)
            {
                if (activeBB == null || inactiveBB == null)
                {
                    Debug.Log("Please assign the bullet button textures, or disable the bullet buttons");
                    break;
                }
                else goto case 5;
            }
            else goto case 5;

        case 5:
            if (imgByPath)
            {
                if (!ImagesInsideProject && !ImagesOutsideProject)
                {
                    Debug.Log("Images by path is selected, but no location has been assigned. Assign the location, or select individual images instead");
                    break;
                }
                //Only occurs if user fiddles with undocumented settings.
                else if (AssignGallery== null)
                {
                    Debug.Log("File path is empty, please assign a location before continuing");
                    break;
                }
                else goto case 6;
            }
            else goto case 6;

        case 6:
            //if the fullscreen function is enabled, check if the textures for the minimize/maximize are set
            if (enbFullscreen)
            {
                if (fullScreenTex == null || fullScreenTexAlt == null)
                {
                    Debug.Log("Full screen has been assigned, but no textures have been. please assign textures for the full screen button, or disable full screen mode.");
                    break;
                }
                else goto case 7;
            }
            else goto case 7;

        case 7:
            CreateGallery();
            break;
    }
}

You don't need a switch here.

The easiest way is to just

if(emptyValueIsDetected)
{
    ShowErrorMessage
    return;
}

like

if (GalleryName == null || GalleryName == "Default")
{
    Debug.Log("No Gallery name assigned, please give the gallery a proper name");
    return;
}
if (gallerySize.x == 0 || gallerySize.y == 0 || gallerySize.width == 0 || gallerySize.height == 0)
{
    Debug.Log("No gallery Sizes assigned, please set some sizes before creating a gallery");
    return;
}

and so on.

Naming

The name indvImages for a variable is not really meaningful. If Mr.Maintainer will inherit this code, he will need some time to understand what the variable should mean.

//Enable Bullet Buttons for the gallery
if (enbBB)

If you need a comment to explain what a variable stands for, you haven't named your variable in a meaningful way. A comment should only describe why something has been done, and not what it is //This is a rectangle.


Find the number of pawns available for a rook to capture I find that very very often with matrix questions, the traversal and comparison part of the solution is very verbose and repetitive.

For example, this code (written for a coding challenge) to find the number of pawns available for a rook to capture in one move on a chess board (R is a white rook, B is a white bishop, and p is a black pawn):

class Solution:
    def numRookCaptures(self, board: List[List[str]]) -> int:
        for r in range(len(board)):
            for c in range(len(board[r])):
                if board[r][c] == 'R':
                    return self.find_p(board, r, c)

    def find_p(self, board, r, c):
        count = 0
        for i in range(r + 1, 8):
            if board[i][c] in ('B', 'p'):
                count += int(board[i][c] == 'p')
                break
        for i in range(r - 1, -1, -1):
            if board[i][c] in ('B', 'p'):
                count += int(board[i][c] == 'p')
                break
        for i in range(c + 1, 8):
            if board[r][i] in ('B', 'p'):
                count += int(board[r][i] == 'p')
                break
        for i in range(c - 1, -1, -1):
            if board[r][i] in ('B', 'p'):
                count += int(board[r][i] == 'p')
                break
        return count

This is the most concise way of writing it that I've found, without compromising clarity. The in for comparison and int for incrementing the count each save us a few lines of code.

But, it's still really verbose and repetitive. Each of the for loops are essentially doing the same exact thing, just in a different direction.

This comes up in every matrix question, whether it's just linear checks like above, BFS, DFS, they pretty much all have a four-directional comparison and traversal that produces 4 copies of code that are all identical in function.

Is there a less verbose way of writing these, without being overly fancy and hard-to-read?

Dimensions

How big is your chess board, and is it square, or at least rectangular?

First, you use range(len(board)) to determine the rows of the chess board, but then you use range(len(board[r])) for the columns, which means you can handle each row having a different number of columns!

(Supporting a non-rectangular chess board would be a challenging challenge, but totally possible, as demonstrated above.)

But then, you use range(r+1, 8) and range(c+1, 8), indicating that you only support square 8x8 chess boards.

Unfortunately, you haven’t posted the challenge problem text, so we can only guess at the actual challenge.  Additional context would go a long way.

Verbosity & Fancyness

In order to be less verbose, we need to do something at least a little bit “fancy”.  With only rook movements, it may seem overly fancy, but if you later support bishops and queens, the additional capability we build in will be leveraged, and very little code need be added.

Additionally, you only need the white bishop blocking the white rook, and only black pawns being captured.  What about white pawns with a black rook and bishop?  This shouldn’t be a special case; it should be generalized.  But again, generalization one one hand will be more verbose, and “fancy”, and the other hand will be leveraged to make the code agnostic to the current player.

First of all, you should have an on_board function, which will return True if a given position is actually on the chess board.  I’ll hard-code the board size, and assume the board is stored in self; the dimensions could be stored in self or derived from self.board on the fly.

def on_board(self, r, c):
    return r in range(8) and c in range(8)

Next, pieces get “blocked” by other pieces.  So let’s formalize that.  Again, I’ll assume the piece is the 'R', so any white (uppercase) piece will block it.   (You should return the appropriate expression based on the colour of piece ... left to student)

def blocked(self, piece, r, c):
    return self.board[r][c].isupper()

A black pawn doesn’t “block” a white rook’s move, but it does terminate it in a “capture” event.  Again, we’ll assume a white piece is moving, so it will “capture” any black (lowercase) piece.  (Again, base actual return value on piece ... left to student)

def capture(self, piece, r, c):
    return self.board[r][c].islower()

Rooks, bishops, and queens can move in a straight line any number of spaces until blocked, or it captures another piece.  Note that a blocked move is not a valid move, but a capture move is a valid move.

def direction_moves(self, piece, r, c, dr, dc):
    r += dr
    c += dc
    while self.on_board(r, c) and not self.blocked(piece, r, c):
        yield r, c
        if self.capture(piece, r, c):
            break
        r += dr
        c += dc

Rooks, bishops and queens can move in more than one direction, so let’s allow multiple directions to be given at once:

def linear_moves(self, piece, r, c, *dirs):
    for dr, dc in dirs:
        yield from self.direction_moves(piece, r, c, dr, dc)

Now that we have the building blocks, getting a rook’s moves from a given position is easy:

def rook_moves(self, piece, r, c):
    yield from self.linear_moves(piece, r, c, (1, 0), (0, 1), (-1, 0), (0, -1))

Moves for the piece at a given position might be:

def moves(self, r, c):
    piece = self.board[r][c]
    if piece in {'R', 'r'}:   # Black or white rook
        yield from self.rook_moves(piece, r, c)
    # ... other types of pieces ...

With this, your find_p becomes trivial:

def find_p(self, r, c):
    return sum(1 for i, j in self.moves(r, c) if self.board[i][j] == 'p')

The above has increased the amount of code, but at the same time it is more generalized.  It has work to be done to generalize it for white -vs- black.  It would be trivial to add movements for bishops and queens to this structure, and that would not be repetitive code, unlike the original structure.

Unfortunately, you may conclude that it has become overly “fancy”.


A flexible Ruby Observable module I was learning a bit about about observers. Ruby has an Observable module, but I decided to make my own with a few tweaks.

The first important feature is that this module calls one or more observer methods when the observed object's state is changed. When you add an observer, you also have to specify the method that will be called (by using a symbol). You can add the same observer again specifying yet another method (so if you add an observer twice (two different methods), when the object's state changes, both methods will be called).

Ruby's Observable module seems to call the #update method on the observer. I prefer to choose the method to be called, so I did that change.

The freedom to associate multiple methods to the same observer (so it is called multiple times per state change) is something I'm unsure if it is useful in any way, though. What do you think about that?

The second important feature is the ability to choose which instance variables (using symbols) will be passed to the observer when it is notified of a state change. This way, when the observer is notified, it won't have to manually read the object's instance variables, since such variables will be passed as arguments to its method.

module Observable

  def add_observer(observer,method,*instance_variables)
    @observers ||= {}
    @observers[observer] ||= []
    entry = [method,instance_variables]
    @observers[observer] << entry unless @observers[observer].include?(entry)
  end

  def remove_observer(observer)
    return unless @observers
    @observers.delete(observer)
  end

  def state_changed
    return unless @observers
    @observers.each do |observer,entries|
      entries.each do |entry|
        variables = []
        entry[1].each do |symbol|
          variables << instance_variable_get("@#{symbol}")
        end
        observer.send(entry[0],*variables)
      end
    end
  end

  def count_observers
    return @observers.size
  end

  alias_method :changed , :state_changed

end

Example:

observableObject.add_observer(self,:somethingChanged,:instanceVar1,:instanceVar2)

When observableObject's state changes, self's method #somethingChanged will be called. And two instance variables from observableObject (instanceVar1 and instanceVar2) will be passed to the method.

What do you think of this implementation? I like the fact that it seems to be more flexible (being able to choose which method to be called and, optionally, what instance variables to pass).

A few notes on making this look more like idiomatic Ruby:

There's no need to use return unless @observers in every method. You initialize @observers to a known value, which will never be falsy; there is no harm at all in assuming it will still be set later.
Any time you initialize an empty array, iterate a collection and (for each item) append something to the array, you're missing out on a map.

Instead of this...

variables = []
entry[1].each do |symbol|
  variables << instance_variable_get("@#{symbol}")
end
observer.send(entry[0],*variables)

...use this:

variables = entry[1].map { |symbol| instance_variable_get("@#{symbol}") }
observer.send(entry[0],*variables)

The last statement in a method is the return value; get rid of your explicit returns:

def count_observers
  @observers.size
end


Finding the difference between two different arrays using JS and FP I'm trying to practice functional programing using JavaScript. These pure functions just work together to find the difference between two arrays. I know that this might be an impractical example of functional programing, but could you let me know where I can improve or maybe where I've misunderstood an FP technique?

"use strict";

let find = (arr1) => (arr2) => {
  let diff1 = findDiff(arr1)(arr2);
  let diff2 = findDiff(arr2)(arr1);

  return concat(diff1)(diff2);
};

let concat = (arr1) => (arr2) => arr1.concat(arr2);
let findDiff = (arr1) => (arr2) => arr1.filter( (elem) => arr2.indexOf(elem) < 0 );

let sweet = find( [1,2,3,4,5] )( [1,2,3,5,6] );
console.log(sweet);

I am concerned that you really haven't defined "difference" here and that perhaps your code does not perform the way you expect.

You example is quite simple in that you would expect [4, 6].  But what if you had arrays like:

[1, 1, 2, 2, 3, 3, 4, 4, 5, 5]
[1, 2, 3, 5, 6]

What do you expect to be returned?  Currently, your function would return [4, 4, 6], but is that what you expect, or would each instance of a repeated value need to be treated differently (i.e. return would be [1, 2, 3, 4, 4, 5, 6])?

Have you considered flipping your array values into object keys (if type conversion is not a concern) or using Array.sort() on input arrays in combination with fromIndex parameter to Array.indexOf() to minimize array iteration caused by Array.indexOf()? This may not be a concern if you don't expect to be diffing large arrays.

Do you really want to support the find()() syntax for this? I don't really see much value in nesting your function calls in this manner vs. just using:

let find = (arr1, arr2) => {
    // function logic
}

You are not ever returning the intermediate function for possible use elsewhere, so I don't follow the need for this approach.

I agree with the comment from @GregBurghardt around not unneccesarily wrapping native array functions.

I don't like the function being called find.  Perhaps something more descriptive to what is actually happening here like arrayDiff(). You are not finding here at all.

To me this whole thing would be clearer/simpler like this:

let arrayDiff = (arr1, arr2) => {
    return arr1.filter( elem => arr2.indexOf(elem) < 0 )
       .concat(
            arr2.filter( elem => arr1.indexOf(elem) < 0 )
       );
};

Note, this obviously does not address some of the questions I asked about uniqueness of array values or optimization of the filter operations.


Making a dataframe of parent IDs I am quite new to scripting, and here is my first code I wrote. The goal of the code is to extract a parent ID based on a child ID. I would appreciate some constructive criticism and aggressive checkup. The codes works and does what it's supposed to do. But how can I make it more pythonic? more passionate?

import codecs

import numpy as np
import pandas as pd

def import_file(path):
    '''imports the .csv files as a pandas dataframe
    Args:
        path (csv file): Takes in the path for the .csv file

    Returns:
        Returns a pandas dataframe
        '''
    with codecs.open(path, "r", encoding='utf-8', errors='ignore') as fdata:
        df = pd.read_csv(fdata)
    return df

def appends_address_before_name(file):
    '''Appends the address before the name ID name
    Returns:
        Returns the file, with address appened to column name.
        '''
    file['ID'] = [address + str(col) for col in file['ID']]
    return file

def create_parent_name(file, column_name: str):
    '''This will create a parent name based on the ID column

    Args:
        file: takes in the dataFrame created from appends_address_before_name function

        column_name: Takes in the column name where the parent name
        will be extracted from.The logic is to split it on the last dot.
        [[parentname].[+ childname]]

    Returns:
        Returns a pandas dataframe with a new column called parentID
        '''
    file['parentID'] = [
        x.rsplit('.', 1)[0] if '.' in x else x[:-1] for x in file[column_name]
    ]

    return file

address = 'New_Jersey_'
file_1 = import_file(r'C:\humans.csv')
file_2= appends_address_before_name(file=file_1)
file_3= create_parent_name(file=file_2 , column_name = 'ID')

print(file_3)

The input CSV is a column of values separated with decimal places like the following:

ID

99.99.9
100.42.3

Example output

parentID

New_Jersey_99.99
New_Jersey_100.42

Furthermore, I feel like the way I am passing variables between the functions at the end of the code seems quite basic and terrible. What can I improve in the above code and how can I improve it ?

A screenshot of the code working

You should decide if your functions modify the object they receive or if the return a modified object. Doing both is just asking for disaster. After your code has finished, file_1, file_2 and file_3 are all identical.

The usual convention is to return None (implicitly or explicitly) if you mutate any of the inputs. In the rest of this answer I have decided to mutate the inputs.

Besides that, pandas is most effective if you use its vectorized functions. For columns with strings, it has a whole lot of methods which are vectorized. You can access them with df.col_name.str. You can find some examples in the documentation.

Your appends_address_before_name function could be simplified a lot because string addition is vectorized:

def appends_address_before_name(file):
    file["parentID"] = address + file["parentID"]
    file["ID"] = address + file["ID"]

And your create_parent_name function could be:

def create_parent_name(file, column_name: str):
    file["parentID"] = file[column_name].str.split(".").str[:-1].str.join(".")

With a csv file like this:

ID
99.99.9
100.42.3
101

This produces

df = import_file(file_name)
create_parent_name(df, 'ID')
appends_address_before_name(df)
print(df)
#                     ID           parentID
# 0   New_Jersey_99.99.9   New_Jersey_99.99
# 1  New_Jersey_100.42.3  New_Jersey_100.42
# 2       New_Jersey_101        New_Jersey_

Note that the order of the calls has changed, so that ids without a . are handled correctly.

As for general structure:

Seeing docstrings is very nice (I omitted them here for brevity)
Python has an official style-guide, PEP8. It recommends writing x = some_thing(a=3), so surround equal signs with spaces when assigning but not when setting keyword arguments.
You should wrap the main calling code in a if __name__ == "__main__" guard.


C++ permutation generator I wrote this code to help with programming challenges. A lot of problems can be solved by complete search so I wrote this function that I can copy/paste between problems to generate permutations quickly. Does anyone have any comments or suggestions to improve this function?

bool bruteforceGenerator(string& permutationIn, const vector<char>& options, const int maxLength)
{
    if(permutationIn.size() == 0) // In case we begin with a blank string just return the starting case
    {
        permutationIn = options[0];
        return true;
    }
    else
    {
        // First try to increment any digit in the string starting from the right
        for(int i = permutationIn.size() - 1; i>=0; i--)
        {
            if(permutationIn[i] != options[options.size()-1]) // If we can increment any digit
            {
                // Increment the individual digit
                auto index = find(options.begin(), options.end(), permutationIn[i]);
                permutationIn[i] = *(index+1);
                // Set all the digits after i back to their start values
                for(int j = i+1; j<permutationIn.size(); j++)
                {
                    permutationIn[j] = options[0];
                }
                return true;
            }
        }

        if(permutationIn.size() < maxLength) // If we could not increment a digit perhaps we could still make the string longer
        {
            permutationIn.push_back(0);
            for(auto &i : permutationIn) i = options[0];
            return true;
        }
        else return false; // If we could not increment any digit and we could not make the string any longer then we are out of permutations
    }
}

The return type of the function was chosen to allow usage of the function to look something like

string permutation;
vector <char> options {'^','v','<','>'};
while(bruteforceGenerator(permutation, options, 2))
{
    // Do something with the permutation
}

Sample output:

^
v
<
>
^^
^v
^<
^>
v^
vv
v<
v>
<^
<v
<<
<>
>^
>v
><
>>

A few things...

Parameter Name

bool bruteforceGenerator(string& permutationIn, ...

From what I can see, this is not an "In" parameter. It's an "InOut". It actually keeps track of the current state too.

Expressions and Spaces

for(int i = permutationIn.size() - 1; i>=0; i--)

Here I'm being picky. You have spaces around the i = ... - 1, but you lose them in the second expression: i>=0. I would suggest that you choose one or the other style. I prefer spaces because I think it makes it easier to read.

Blocks of code

else return false;

On this one, long term, I prefer to have a block like so:

else
{
    return false;
}

It's also a lot easier to not later make a mistake by adding a block after the else and forgetting to move the return false; statement. Also always having a block means you can add a second line of code and it will probably work as expected. Without the block, it probably won't work as expected. Note that applies to this statement as well:

for(auto &i : permutationIn) i = options[0];

Lighter/Clearer overall Function

Finally, as a whole, I would change the way the function if/else blocks are managed. I would actually not have an else and would have a return false; as the very last statement. There is sample skeleton:

if(condition)
{
    ...
    return true;
}

for(...;...;...)
{
    if(condition)
    {
        ...
        return true;
    }
}

if(condition)
{
    return true;
}

return false;

I think that makes it simpler to read. The cases that work are managed first and return true. If nothing works, we end up returning false.

There is a comment by @Incomputable about an std::next_permutation algorithm. I can see that your algorithm would not fit for two reasons:

The std::next_permutation requires you start with a sorted array of characters; and
It only computes permutations with a size equal to the input size when in your case you start with 1 character and grow up to maxlength which can be smaller than the input length (as in your example).

However, one interesting aspect of that algorithm is the fact that you can run through the permutations again after false is returned.

You could achieve that behavior by clearing the permutationIn variable just before returning false:

    ...
    permutationIn.clear();
    return false;
}

Of course, the pattern you have is similar to reading a file. Once the EOF is found, further reading continues to give you EOF. Either pattern has its merits...


Assign a unique number to duplicate items in a list I made a program, but I not happy with the quantity of code. The end result is good, but I believe it can be made much easier, only I don't know how.

The functionality: If equal items > 1 in list, then assign all equal items a unique set number.  Below I made a unit test. I'm not happy with the class CreatSet. Can somebody advise me on how this can be implemented better?

import unittest

class Curtain(object):
  def __init__(self, type, fabric, number):
      self.type = type
      self.fabric = fabric
      self.number = number
      self.set_number = None

  def __str__(self):
      return '%s %s %s %s' % (self.number, self.type, self.fabric, self.set_name)

  def __eq__(self, other):
      return self.type == other.type and self.fabric == other.fabric

class CreatSet(object):
  def make_unique(self, original_list):
      checked = []
      for e in original_list:
          # If curtain: type and fabric is equal
          if e not in checked:
              checked.append(e)
      return checked

  def create_set(self, curtains):
      # Uniuqe items in list
      unique_list = self.make_unique(curtains)
      result = []
      for x in unique_list:
          # Create set list
          set_range = []
          for y in curtains:
              if y == x:
                  set_range.append(y)
          # Add set range into list
          result.append(set_range)

      # Create set number
      set_result = []
      set_number = 0
      for x in result:
          if len(x) == 1:
              set_result.append(x[0])
          else:
              set_number += 1
              for y in x:
                  y.set_number = set_number
                  set_result.append(y)
      # Return list ordered by number
      return sorted(set_result, key=lambda curtain: curtain.number)

class TestCreateSet(unittest.TestCase):
    def setUp(self):
      self.curtains = []
      self.curtains.append(Curtain('pleatcurtain', 'pattern', 0))
      self.curtains.append(Curtain('pleatcurtain', 'plain', 1))
      self.curtains.append(Curtain('pleatcurtain', 'pattern', 2))
      self.curtains.append(Curtain('foldcurtain', 'pattern', 3))
      self.curtains.append(Curtain('pleatcurtain', 'plain', 4))
      self.curtains.append(Curtain('foldcurtain', 'plain', 5))
      self.curtains.append(Curtain('pleatcurtain', 'pattern', 6))
      self.curtains.append(Curtain('foldcurtain', 'pattern', 7))

    def test_auto_set(self):
      creat_set = CreatSet()
      result = creat_set.create_set(self.curtains)
      # Creating set
      self.assertEqual(result[0].set_number, 1)  # pleatcurtain, pattern
      self.assertEqual(result[1].set_number, 2)  # pleatcurtain, plain
      self.assertEqual(result[2].set_number, 1)  # pleatcurtain, pattern
      self.assertEqual(result[3].set_number, 3)  # foldcurtain, pattern
      self.assertEqual(result[4].set_number, 2)  # pleatcurtain, plain
      self.assertEqual(result[5].set_number, None)  # foldcurtain, plain
      self.assertEqual(result[6].set_number, 1)  # pleatcurtain, pattern
      self.assertEqual(result[7].set_number, 3)  # foldcurtain, pattern

if __name__ == '__main__':
    unittest.main()

As you've already implemented Curtain.__eq__, if you also implement Curtain.__hash__ you can use it as a dictionary (or collections.Counter...) key, or a set member:

def __hash__(self):
    return hash(self.type) ^ hash(self.fabric)

Now make_unique is trivial:

def make_unique(self, original_list):
    return set(original_list)

(Note: if you require order to be retained, this will need additional work.)

This also allows you to easily determine how many of each distinct Curtain you have:

>>> from collections import Counter
>>> Counter(curtains)
Counter({<__main__.Curtain object at 0x02A15190>: 3,
         <__main__.Curtain object at 0x031A0CF0>: 2,
         <__main__.Curtain object at 0x031A0ED0>: 2,
         <__main__.Curtain object at 0x0329EA30>: 1})

(Note: implementing Curtain.__repr__ would make this more readable!)

As pointed out in the comments, the __hash__ implementation I suggest has an issue - if either attribute type or fabric is changed, the hash will be different. You could protect these attributes by making them "read-only" using properties:

class Curtain(object):

    def __init__(self, type, ...):
        self._type = type # note leading underscore on attribute

    @property # defining setter but no getter
    def type(self):
        return self._type

Alternatively, you can implement Curtain.__lt__ etc., then sort the list and use itertools.groupby to get your groups of "equal" Curtains.

Either way, I would not implement CreatSet as a class, there's no need (as should be clear from the fact that there is no __init__ and no class or instance attributes). Just have one class, and two functions:

class Curtain(object):
    ...

def create_set(curtains):
    ...

def make_unique(curtains):
    ...

Your code is generally compliant with the style guide (well done!) but you could do with some explanatory docstrings.


Create a list of all strings within hamming distance of a reference string with a given alphabet For a bioinformatic problem, I found myself wanting to create a list of strings within Hamming distance "k" of a reference sequence. I wanted to do so quickly and pythonically. I've implemented it in pure python and in cython, with and without type declarations. The time performance has been identical. (I also compared the compiled python version with an interpreted version defined in ipython, which also performed identically.)

cfi has been set as shorthand to chain.from_iterable in order to reduce the number of dot operators being used as in the following module-level import and definition:

from itertools import chain
cfi = chain.from_iterable

@cython.returns(list)
def PermuteMotifOnce(cython.str motif, set alphabet={"A", "C", "G", "T"}):
    """
    Gets all strings within hamming distance 1 of motif and returns it as a
    list.
    """
    return list(set(cfi([[
        motif[:pos] + alpha + motif[pos + 1:] for
        alpha in alphabet] for
                         pos in range(len(motif))])))

def PyPermuteMotifOnce(motif, alphabet={"A", "C", "G", "T"}):
    """
    Gets all strings within hamming distance 1 of motif and returns it as a
    list.
    """
    return list(set(cfi([[
        motif[:pos] + alpha + motif[pos + 1:] for
        alpha in alphabet] for
                         pos in range(len(motif))])))

@cython.returns(list)
def PermuteMotifN(cython.str motif, cython.long n=-1):
    assert n > 0
    cdef set workingSet
    cdef cython.long i
    workingSet = {motif}
    for i in range(n):
        workingSet = set(cfi(map(PermuteMotifOnce, workingSet)))
    return list(workingSet)

def PyPermuteMotifN(motif, n=-1):
    assert n > 0
    workingSet = {motif}
    for i in range(n):
        workingSet = set(cfi(map(PermuteMotifOnce, workingSet)))
    return list(workingSet)

Results:

motif = "ACCTACTGAACT"
%timeit -n 5 PermuteMotifN(motif, 6)
5 loops, best of 3: 6.93s per loop
%timeit -n 5 PyPermuteMotifN(motif, 6)
5 loops, best of 3: 6.81s per loop
%timeit -n 5000 PyPermuteMotifN(motif, 2)
5000 loops, best of 3: 589 microseconds per loop
%timeit -n 5000 PermuteMotifN(motif, 2)
5000 loops, best of 3: 645 microseconds per loop

Is it just me, or does the pure Python seem even faster than the Cython? Is there a lot of time lost in extra type checking?

Your strategy here is to keep a working set and repeatedly expand it by adding strings within Hamming distance 1 of elements in the set. This involves a lot of duplicated work, because the same candidate strings are going to be generated many times over (and then thrown away because they are already members of the working set).

I find that I get a substantial speedup just by generating each string exactly once.

from itertools import chain, combinations, product

def hamming_circle(s, n, alphabet):
    """Generate strings over alphabet whose Hamming distance from s is
    exactly n.

    >>> sorted(hamming_circle('abc', 0, 'abc'))
    ['abc']
    >>> sorted(hamming_circle('abc', 1, 'abc'))
    ['aac', 'aba', 'abb', 'acc', 'bbc', 'cbc']
    >>> sorted(hamming_circle('aaa', 2, 'ab'))
    ['abb', 'bab', 'bba']

    """
    for positions in combinations(range(len(s)), n):
        for replacements in product(range(len(alphabet) - 1), repeat=n):
            cousin = list(s)
            for p, r in zip(positions, replacements):
                if cousin[p] == alphabet[r]:
                    cousin[p] = alphabet[-1]
                else:
                    cousin[p] = alphabet[r]
            yield ''.join(cousin)

def hamming_ball(s, n, alphabet):
    """Generate strings over alphabet whose Hamming distance from s is
    less than or equal to n.

    >>> sorted(hamming_ball('abc', 0, 'abc'))
    ['abc']
    >>> sorted(hamming_ball('abc', 1, 'abc'))
    ['aac', 'aba', 'abb', 'abc', 'acc', 'bbc', 'cbc']
    >>> sorted(hamming_ball('aaa', 2, 'ab'))
    ['aaa', 'aab', 'aba', 'abb', 'baa', 'bab', 'bba']

    """
    return chain.from_iterable(hamming_circle(s, i, alphabet)
                               for i in range(n + 1))

In pure Python, this runs about 4 times as fast as the code in the original post.


Calculate Total of Items in List I would like to convert the following procedure to use Linq, but I'm not sure how to accomplish it, ForEach or Sum?

    public int GetBalance()
    {
        int balance = 0;

        foreach (IDenomination denomination in Account)
        {
            balance += denomination.Currency * denomination.BillCount;
        }

        return balance;
    }

Account is a property in the class:

public List<IDenomination> Account { get; set; }

Or you could do it in two steps, like you do it now. First calculate for each denomination and then sum:

int balance = Account.Select(denom => denom.Currency * denom.BillCount).Sum();


Basic COBOL Calculator Small Calculator I made for myself while trying to learn COBOL for fun. Compiler used is GNUCOBOL. Just asks for input and will either do multiplication, subtraction, or addition. Completes run after input is asked.
MATH
       IDENTIFICATION DIVISION.
       PROGRAM-ID. MATH.

       DATA DIVISION.
       WORKING-STORAGE SECTION.
       01 INP PIC A(14).
       01 NUM1 PIC 9(7).
       01 NUM2 PIC A(2).
           88 PLU VALUE "+".
           88 AIDS VALUE "-".
           88 MULT VALUE "*".
       01 NUM3 PIC 9(15).
       01 ANS PIC 9(8).
       PROCEDURE DIVISION.
           DISPLAY "Input: "
           ACCEPT INP.
           UNSTRING INP DELIMITED BY SPACE
               INTO NUM1, NUM2, NUM3
           END-UNSTRING.
           IF NUM2 = "+" THEN
               ADD NUM1 NUM3 GIVING ANS
           ELSE
               IF NUM2 = "-" THEN
                   SUBTRACT NUM1 FROM NUM3 GIVING ANS
               ELSE
                   IF NUM2 = "*" THEN
                       MULTIPLY NUM1 BY NUM3 GIVING ANS
           END-IF.
           DISPLAY ANS.
           STOP RUN.

COBOL documentation isn't as great compares to languages like C++ or Javascript, so I struggled a bit.

First off, you don't verify the data entered by the user. This is an absolute must, IMHO.
I'm no COBOL programmer, but do have some basic COBOL knowledge. Others may suggest better solutuions. Anyway, here are my thoughts:
You define data item 01 INP PIC A(14) but the user needs to input numbers and operators. Picture character A means *letter of the Latin alphabet or a space", You should use X, because you cannot tell what the user will enter, and that is why you need to add code to verify.
You define the data items to hold the input numbers as PIC 9(7), and PIC 9(15), resp., and the data item to hold the operation result as PIC 9(8). How did you come up with these lengths? Make sure the result fits into the corresponding data item.
You define the data item to hold the math operator as PIC A(2), but the operators are only one character (and not of type A). Again PIC X would be appropriate.
You're defining condition names for the operators, which is a good idea.
   01 NUM2 PIC A(2).
      88 PLU VALUE "+".
      88 AIDS VALUE "-".
      88 MULT VALUE "*".

However, you don't make user of them. You can use them in your IF statements, and in the (missing) logic to verify the input. So instead of
   IF NUM2 = "+" THEN
       ADD NUM1 NUM3 GIVING ANS
   ELSE ...

you could code
   IF PLU THEN
       ADD NUM1 NUM3 GIVING ANS
   ELSE ...

Instead of coding nested IF statements use other constructs the language of choice offers. In COBOL, you could make use of the EVALUATE statement as follows:
   EVALUATE TRUE
      WHEN PLU
         ADD NUM1 NUM3 GIVING ANS
      WHEN AIDS
         SUBTRACT NUM1 FROM NUM3 GIVING ANS
      WHEN MULT
         MULTIPLY NUM1 BY NUM3 GIVING ANS
      WHEN OTHER
         some error processing here....
    END-EVALUATE.


Calculate the sum of the square of two biggest numbers out of three I'm reading Structure and Interpretation of Computer Programs.  In the first exercise, I came across a programming problem to add the squares of the 2 bigger numbers out of 3 numbers.

I came up with the following procedure:

(define (sum-of-larger-two a b c)
  (
    if (> a b)
      (if (> b c)
        (+ (* a a) (* b b))
        (+ (* a a) (* c c))
      )
      (if (> c a)
        (+ (* b b) (* c c))
        (+ (* b b) (* a a))
      )
  )
)

Is there a more succinct way to write this program?

One of the ways of obtaining concision is by “astracting” a repeated operation to define a new function. For instance in this case you could use the square function previously defined in that chapter.

But if you look at the expression (square a) you will discover that it is longer then (* a a), even it is, in some sense, more readable (has a minor number of symbols, so it is simpler to understand).

But having a function to abstract an operation could have other benefits that an immediate reduction in the count of the characters. Consider that in your example you are comparing two numbers to find the larger between them: in other words, you are looking for their maximum.

So, let's define  a new maximum operator:

(define (max a b)
  (if (> a b) a b)

Then, having the functions square and max you can now combine them, like, for instance, in:

(define (sum-of-larger-two a b c)
  (if (> a b)
      (+ (square a)
         (square (max b c)))
      (+ (square b)
         (square (max a c)))))

Or you can rearrange the operations in this way:

(define (sum-of-larger-two a b c)
  (+ (square (max a b))
     (square (if (= (max a b) a)
                 (max b c)
                 (max a c)))))

Note that we are reducing the number of conditions to check in the function sum-of-larger-two (in addition to reducing the number of lines and symbols), and so we are simplifying it, improving both its readability as well as its maintainability.


Mapping enum keys to instance values I am working on a pathfinding program / algorithm, and I have the following class:

[System.Serializable]
public class UnitTileCosts
{
    int[] tileCosts;

    public UnitTileCosts()
    {
        //empty constructor uses default value of 1 for all fields
        GenerateDefaults();
    }

    public UnitTileCosts(Dictionary<TileType,int> costs)
    {
        GenerateDefaults();
        foreach(var pair in costs)
        {
            tileCosts[(int) pair.Key] = pair.Value;
        }
    }

    void GenerateDefaults()
    {
        tileCosts = new int[System.Enum.GetNames(typeof(TileType)).Length];
        for(int i = 0; i < tileCosts.Length; i++)
        {
            tileCosts[i] = 1;
        }
    }

    public int GetTileCost(TileType tile)
    {
        return tileCosts[(int) tile];
    }
}

Is this better or worse than simply using a Dictionary<TileType, int> in the first place?

Using the Dictionary (that is passed in to the constructor), I would need to check if the key exists because I only care about those values that differ from the result. (Which, admittedly, is easily doable with TryGetValue and an OUT parameter)

I would think that an array of ints would serialize better than a dictionary, to boot.

Is casting from the enum to the int acceptable in this instance? I understand that enums are designed to avoid magic numbers, but in this case I only care about the numbers (and not the magic), so I think it might be reasonable.

I'm not sure if you're gaining anything from this wrapper class or not, but there is an alternative for initializing an array to have all the same value at each position. You could use Enumerable.Repeat instead of the loop.

void GenerateDefaults()
{
    tileCosts = Enumerable.Repeat(1, System.Enum.GetNames(typeof(TileType)).Length).ToArray();
}

But perhaps the one liner is a bit too terse to be readable.

void GenerateDefaults()
{
    var length = System.Enum.GetNames(typeof(TileType)).Length;

    tileCosts = Enumerable.Repeat(1, length).ToArray();
}

Whether or not that's an improvement may be debatable and is mostly a matter of preference in my opinion.

Something else I don't care for is the fact that GenerateDefaults is a void. As such, it's not immediately clear that it modifies the tileCosts class variable. I would redefine it to return int[]. It's a few more keystrokes to call it, but it becomes obvious where tileCosts is being modified.

[System.Serializable]
public class UnitTileCosts
{
    int[] tileCosts;

    public UnitTileCosts()
    {
        //empty constructor uses default value of 1 for all fields
        tileCosts = GenerateDefaults();
    }

    public UnitTileCosts(Dictionary<TileType,int> costs)
    {
        tileCosts = GenerateDefaults();
        foreach(var pair in costs)
        {
            tileCosts[(int) pair.Key] = pair.Value;
        }
    }

Which begs the question, why set a default value at all if you're just going to overwrite the value a split second later?? It doesn't make sense to me and I would remove that code. At the least, I would make the overloaded constructor call the default constructor to dry things up.

    public UnitTileCosts(Dictionary<TileType,int> costs) : this()
    {
        foreach(var pair in costs)
        {
            tileCosts[(int) pair.Key] = pair.Value;
        }
    }


Generate JSON for Map>> Consider I have a String:

String value = "Sunday  -  H15,Sunday  -  H03,Sunday  -  H13,Sunday  -  H01,Sunday  -  H05,Friday  -  H23,Saturday  -  H05,Monday  -  H16,Monday  -  H17,Monday  -  H18,Monday  -  H19,Monday  -  H20";

In this String we have Days (Sunday, Monday etc..) and the alphanumeric (H15, H03 etc..) is hours.

Now I want a JSON format for Map<String, Map<String, List<String>>> in which Key should be Day and Value should be the Map having Key as Hours and Values as list of Hours which has same day. Something like this format:

{"Monday":{"Hours":["H16","H17","H18","H19","H20"]},"Thursday":{"Hours":[]},"Friday":{"Hours":["H23"]},"Sunday":{"Hours":["H15","H03","H13","H01","H05"]},"Wednesday":{"Hours":[]},"Tuesday":{"Hours":[]},"Saturday":{"Hours":["H05"]}}

Note: In this example there are some days with valid hours (like Sunday, Monday etc) while other are having empty Hours (like Wednesday and Thursday).

This code is working fine as required but I feel that it can be refactored.

public class DayPartProcessingTest {

  public static void main(String[] args) throws JsonProcessingException {
    String propertyValue = "Sunday  -  H15,Sunday  -  H03,Sunday  -  H13,Sunday  -  H01,Sunday  -  H05,Friday  -  H23,Saturday  -  H05,Monday  -  H16,Monday  -  H17,Monday  -  H18,Monday  -  H19,Monday  -  H20";
    Map<String, List<String>> map = new HashMap<String, List<String>>();
    String[] dayParts = propertyValue.split(",");

    for (String string : dayParts) {
      String[] array = string.split("  -  ");
      if (map.get(array[0]) == null) {
        List<String> list = new ArrayList<>();
        list.add(array[1]);
        map.put(array[0], list);
      } else {
        List<String> tempList = map.get(array[0]);
        tempList.add(array[1]);
        map.put(array[0], tempList);
      }
    }

    Map<String, Map<String, List<String>>> payload = new HashMap<>();
    Map<String, List<String>> hrs1 = new HashMap<>();
    hrs1.put("Hours", new ArrayList<String>());
    payload.put("Sunday", hrs1);
    payload.put("Monday", hrs1);
    payload.put("Tuesday", hrs1);
    payload.put("Wednesday", hrs1);
    payload.put("Thursday", hrs1);
    payload.put("Friday", hrs1);
    payload.put("Saturday", hrs1);
    for (Entry<String, List<String>> entry : map.entrySet()) {
      Map<String, List<String>> hrs = new HashMap<>();
      hrs.put("Hours", entry.getValue());
      if (payload.containsKey(entry.getKey())) {
        payload.put(entry.getKey(), hrs);
      }
    }

    String json = new ObjectMapper().writeValueAsString(payload);
    System.out.println(json);

  }

Methods

The first thing that we can do is divide up the labor.  Consider

void static main(String[] args) throws JsonProcessingException {
    Map<String, List<String>> properties = parseProperties(getPropertyValue());
    Map<String, Map<String, List<String>>> payload = buildPayload(properties);
    String json = new ObjectMapper().writeValueAsString(payload);
    System.out.println(json);
}

Now we have the heavy lifting in parseProperties and buildPayload.

I also hid the propertyValue in a method, but that was mostly to avoid obnoxious horizontal scrolling.

parseProperties

    String[] dayParts = propertyValue.split(",");

    for (String string : dayParts) {
      String[] array = string.split("  -  ");
      if (map.get(array[0]) == null) {
        List<String> list = new ArrayList<>();
        list.add(array[1]);
        map.put(array[0], list);
      } else {
        List<String> tempList = map.get(array[0]);
        tempList.add(array[1]);
        map.put(array[0], tempList);
      }
    }

Consider

    for (String string : propertyValue.split(",")) {
      String[] parts = string.split("  -  ");

      List<String> hours = map.get(parts[0]);
      if (hours == null) {
        hours = new ArrayList<>();
      }

      hours.add(parts[1]);
      map.put(parts[0], hours);
    }

We don't need a separate variable for the results of the comma split.  We only use that for this loop, so we can do it directly.

We do need a separate variable for the parts of that string, as we use the parts separately.  Also, we use one of them multiple times.

In both branches of the original if, we do two of the same commands.  If we rewrite it this way, we can do those outside the if.

In the original if, we could have used contains, as we don't use the value otherwise.  But this way, we save the result.  If it is empty, we put an empty list there.  Otherwise, we can just use the previous value.

buildPayload

    Map<String, Map<String, List<String>>> payload = new HashMap<>();
    Map<String, List<String>> hrs1 = new HashMap<>();
    hrs1.put("Hours", new ArrayList<String>());
    payload.put("Sunday", hrs1);
    payload.put("Monday", hrs1);
    payload.put("Tuesday", hrs1);
    payload.put("Wednesday", hrs1);
    payload.put("Thursday", hrs1);
    payload.put("Friday", hrs1);
    payload.put("Saturday", hrs1);
    for (Entry<String, List<String>> entry : map.entrySet()) {
      Map<String, List<String>> hrs = new HashMap<>();
      hrs.put("Hours", entry.getValue());
      if (payload.containsKey(entry.getKey())) {
        payload.put(entry.getKey(), hrs);
      }
    }

So you want to create a map with the seven days of the week.  The same pattern can work here.

public static Map<String, List<String>> normalizeHours(Map<String, List<String>> hours) {
    if (hours == null) {
        hours = new HashMap<>();
        hours.put("Hours", new ArrayList<String>());
    }

    return hours;
}

And we'd use it like

    Map<String, Map<String, List<String>>> payload = new HashMap<>();
    payload.put("Sunday", normalizeHours(properties.get("Sunday")));
    payload.put("Monday", normalizeHours(properties.get("Monday")));
    payload.put("Tuesday", normalizeHours(properties.get("Tuesday")));
    payload.put("Wednesday", normalizeHours(properties.get("Wednesday")));
    payload.put("Thursday", normalizeHours(properties.get("Thursday")));
    payload.put("Friday", normalizeHours(properties.get("Friday")));
    payload.put("Saturday", normalizeHours(properties.get("Saturday")));

Instead of putting a dummy value into each day of the week and then replacing it, this puts the correct value in in the first place.

It's not shorter but is more reusable to have a container for the days of the week that you can use here.

    for (String dayOfTheWeek : dayNames) {
        payload.put(dayOfTheWeek, normalizeHours(properties.get(dayOfTheWeek)));
    }

It's not much longer even for this single problem.  Once you have the container, it shortens the code.


Allocating a contiguous block of memory for an array I try to work with a contiguous block of memory, to create an array (2D) whose SIZE is not known at compile time (before c99) so no variable-length arrays are involved here.

I came up with the following:

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

int main(void){
    unsigned int row, col,i, j, k;
    int l = 0;
    int *arr;

    printf("Give the ROW: ");
    if ( scanf("%u",&row) != 1){
        printf("Error, scanf ROW\n");
        exit(1);
    }

    printf("Give the COL: ");
    if ( scanf("%u",&col) != 1){
        printf("Error, scanf COL\n");
        exit(2);
    }

    arr = malloc(sizeof *arr * row * col);
    if(arr == NULL){
        printf("Error, malloc\n");
        exit(3);
    }

    for ( i = 0; i < row ; i++){
        for ( j = 0 ; j < col ; j++){
            arr[i * col + j] = l;
            l++;
        }
    }

    for (k = 0 ; k < (row * col) ; k++){
        printf("%d ",arr[k]);
    }

    free(arr);
}

Which gives me the following:

Give the ROW: 6
Give the COL: 3
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17

Is this the right approach?

Of course this in not a true 2d array, just a 1d array indexed using arr[i * col + j].

This is an OK approach.  When able, I prefer an array of pointers to an array of ints, as it is usually more flexible to the tasks I tackle.  YMMV.

Modest extreme case: sizeof *arr * row * col is multiplied in the right order.  When row or col is int/unsigned, good to use sizeof *arr first as that is type size_t and the subsequent multiplication will then happen at least using size_t math.   The concern being that with unsigned row, col, row*col  calculated first could overflow using unsigned math, but not size_t math.

Pedantically, even sizeof *arr * row * col can overflow.  Should code need to detect this, the reasonable approach is to use the widest unsigned types like unsigned long long or uintmax_t.  (Other more pedantic methods exist.)

assert(1LLU * sizeof *arr * row * col < SIZE_MAX);

Suggest using size_t for row, col, i, j instead of any other type.  size_t is the Goldilocks type for array indexes - not too narrow, not too wide.
Error messages are better sent to stderr.  2 advantages: will not get lost in code's typical stdout output and the stderr stream is flushed ensuring output is not mixed after later input.
Idea: Using a arr = 0; after the free(arr); has merit when the free'd variable is still in scope for a long time.  Although not needed, a decent compiler will optimized it out, but errant subsequent use is easier to detect when arr == NULL than arr = some_freed_value.
Minor: printf("%d ",arr[k]); in a loop is sometimes a problem when trying to print more than some environmental limit number (e.g. 4095) of characters on the same line.  Liberal use of fflush(stdout); or including a '\n' in the text usually solves this.
A key comment missing is: is this array row or column major?  With some analysis, that is discernible, but from a header only interface, knowing which dimension moves in the smallest steps is important in how higher level code will want to use this array for cache hit efficiency.
Coding style:  No need to introduce a variable until ready for assignment.  Consider the 2nd style.

// int *arr;
// ... many lines
// arr = malloc(sizeof *arr * row * col);

... many lines
int *arr = malloc(sizeof *arr * row * col);


Get params from a URL I want to write my own version of HttpUtility.ParseQueryString, because it is not included in the .NET Framework Client Profile. My code is pretty straightforward and simple, but I'm looking at the code of HttpUtility.ParseQueryString with Reflector, and is about 200 lines including the code of the called methods.

I don't want to just steal the code specially because there are parts that I have no idea what they are for. Is my code OK or am I missing something (I don't care about performance here)?

Dictionary<string, string> GetParams(string str)
{
    int questionMarkIndex = str.IndexOf('?');
    if (questionMarkIndex != -1)
    {
        string paramsString = str.Substring(questionMarkIndex + 1);
        string[] keyValues = paramsString.Split(new char[] { '&' }, StringSplitOptions.RemoveEmptyEntries);
        Dictionary<string, string> result = new Dictionary<string, string>();

        foreach (string keyValue in keyValues)
        {
            string[] splitKeyValue = keyValue.Split(new char[] { '=' });
            if (splitKeyValue.Length == 2 && !string.IsNullOrWhiteSpace(splitKeyValue[0]))
            {
                string unencodedKey = Uri.UnescapeDataString(splitKeyValue[0]);
                string unencodedValue = Uri.UnescapeDataString(splitKeyValue[1]);
                result[unencodedKey] = unencodedValue;
            }
        }

        return result;
    }
    else return new Dictionary<string, string>();
}

I figure HttpUtility.ParseQueryString is just incredibly robust, and you likely are missing something. But I can't spot anything in particular and it's likely that without deep research you won't be able to cover all the cases which were clearly studied and thought out in Microsoft's implementation. However, I think your implementation can be refactored to be easier to read and maintain by using regular expressions instead of brute force string splitting. I present the following in F# which hopefully you can translate easily enough to C# (the key functionality relies on the same .NET libraries available in both languages). I'm still new to regex writing myself, so I'm sure it's not as robust as it could be, but it seems to work well for all the typical cases I tested (generally assumes query string is not malformed, allows no query string, allows empty values but not empty keys, accounts for possible # delimiter after query string part):

open System
open System.Text.RegularExpressions
let uriParams uri =
    let matches = Regex.Matches(uri, @"[\?&](([^&=]+)=([^&=#]*))", RegexOptions.Compiled)
    seq { //create a sequence of key, value tuples extracted from the matches
        for m in matches do
            yield Uri.UnescapeDataString(m.Groups.[2].Value), Uri.UnescapeDataString(m.Groups.[3].Value)
    } |> dict // convert the sequence of key, value tuples into an immutable IDictionary

Update

I went ahead and translated to C#:

using System;
using System.Text.RegularExpressions;
using System.Collections.Generic;

...

static Dictionary<string, string> GetParams(string uri)
{
    var matches = Regex.Matches(uri, @"[\?&](([^&=]+)=([^&=#]*))", RegexOptions.Compiled);
    var keyValues = new Dictionary<string, string>(matches.Count);
    foreach (Match m in matches)
        keyValues.Add(Uri.UnescapeDataString(m.Groups[2].Value), Uri.UnescapeDataString(m.Groups[3].Value));

    return keyValues;
}

Update 2

Here's a more functional C# version:

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

...

static Dictionary<string, string> GetParams(string uri)
{
    var matches = Regex.Matches(uri, @"[\?&](([^&=]+)=([^&=#]*))", RegexOptions.Compiled);
    return matches.Cast<Match>().ToDictionary(
        m => Uri.UnescapeDataString(m.Groups[2].Value),
        m => Uri.UnescapeDataString(m.Groups[3].Value)
    );
}

Edit

I updated the regex to address the very important consideration of the # delimiter that @Visionary pointed out. That is probably the biggest thing missing from your own implementation functionality-wise.  Also, here is a list of some cases I tested (using the F# version):

uriParams "http://www.foo.com" // []
uriParams "http://www.foo.com?" // []
uriParams "http://www.foo.com?q=" // [("q","")]
uriParams "http://www.foo.com?q=search&doit=no" // [("q","search"), ("doit","no")]
uriParams "http://www.foo.com?q=search&doit=no&go=further" // [("q","search"), ("doit","no"), ("go","further")]
uriParams "http://www.foo.com?q=&doit=no" // [("q",""), ("doit","no")]
uriParams "http://www.foo.com?q=&doit=no&=&test&good=true" //[("q",""), ("doit","no"), ("good","true")]
uriParams "http://www.foo.com?q=search&doit=no#validationError"  // [("q","search"), ("doit","no")]
uriParams "http://www.foo.com?q=search&doit=no#validationError=true"  // [("q","search"), ("doit","no")]
uriParams "http://www.foo.com?q=se%20arch&do%20it=no" // [("q","se arch"), ("do it","no")]
uriParams "http://www.foo.com?q=search&doit=#validationError"  // [("q","search"), ("doit","")]


OOP Web-scraper w/ Python and BeautifulSoup This is my first major web scraping program in python. My code works nonetheless, I'm just not sure if it's the best OOP design. My code is below:

from bs4 import BeautifulSoup
import requests
import argparse
import sys

class ComicScraper():  # Class ComicScraper for scraping comic books
    def __init__(self, comic_titles, comic_prices, all_comics):
        self.comic_titles = comic_titles
        self.comic_prices = comic_prices
        self.all_comics = all_comics
        # url of comicbook site
        self.url = 'https://leagueofcomicgeeks.com/comics/new-comics/2020/'
        self.webpage = requests.get(self.url)  # HTTP request for url
        # BeautifulSoup object of webpage
        self.soup = BeautifulSoup(self.webpage.content, 'html.parser')
        self.titles = list(
            map(BeautifulSoup.get_text, self.soup.find_all('div', class_='comic-title')))
        self.comicinfo = [x.replace(u'\xa0', u'').strip()
                          for x in list(map(BeautifulSoup.get_text, self.soup.find_all('div', class_='comic-details comic-release')))
                          ]
        self.prices = [
            prices[-5:] if prices[-5:].startswith('$') else 'No price' for prices in self.comicinfo]

    def main(self):
        if len(sys.argv) == 1:
            print("###### New Comics ######")
            for title, info in zip(self.titles, self.comicinfo):
                print(title, '--->', info)
        if self.all_comics:
            print("###### New Comics ######")
            for titles, info in zip(self, titles, self.comicinfo):
                print(title, '--->', info)
        if self.comic_titles and self.comic_prices:
            print("###### New Comics ######")
            for title, prices in zip(self.titles, self.prices):
                print(title, '--->', info)
        if self.comic_titles:
            for comic_title in self.comic_titles:
                print(comic_title)
        if self.comic_prices:
            for dol_amount in comic_prices:
                print(dol_amount)

if __name__ == '__main__':
    parser = argparse.ArgumentParser()
    # Titles of comicbooks i.e "Detective Comics #1"
    parser.add_argument('-t', '--titles', help='Print comic titles ONLY', dest='titles')
    # Scrape prices of comic books in order
    parser.add_argument('-m', '--prices', help='Get comic prices ONLY', dest='prices')

    parser.add_argument('-a', '--all', help='Get titles, prices, publisher, and descriptions',
                        dest='all_comics', action='store_true')
    args = parser.parse_args()
    scraper = ComicScraper(args.titles, args.prices, args.all_comics)
    scraper.main()

I have some doubts about how much instance variables I've used? Would refactoring this code as a bunch of functions be the best way?

Bugs

Lines 32 and 42:

        for titles, info in zip(self, titles, self.comicinfo):

        for dol_amount in comic_prices:

both have unresolved variable references - titles and comic_prices.

Bare class declaration

class ComicScraper():  # Class ComicScraper for scraping comic books

can be

class ComicScraper:
    """
    For scraping comic books
    """

Note the more common format for docstrings used.

Doing too much in an init

requests.get(self.url)

should probably not be done in an __init__. Constructors are usually best to initialize everything that the class will need without "doing" too much.

Argument names

titles doesn't actually accept multiple titles; it only accepts one. Providing the argument twice overwrites the first value. That means that this loop:

        for comic_title in self.comic_titles:

is actually going to loop through each of the characters in the provided string, which is probably not what you want.

Reading the docs - https://docs.python.org/3/library/argparse.html - you probably want action='append'.

Reaching past argparse

This:

    if len(sys.argv) == 1:

should not be done. Instead, rely on the output of argparse.

Suggested

Here is a re-thought program:

from bs4 import BeautifulSoup, Tag
from datetime import date, datetime
from typing import Iterable
import argparse
import re
from requests import Session

class Comic:
    # ·  Apr 8th, 2020  ·  $7.99
    RELEASE_PAT = re.compile(
        r'^\s*·\s*'
        r'(?P<month>\S+)\s*'
        r'(?P<day>\d+)\w*?,\s*'
        r'(?P<year>\d+)\s*'
        r'(·\s*\$(?P<price>[0-9.]+))?\s*$'
    )

    def __init__(self, item: Tag):
        self.id = int(item['id'].split('-')[1])
        sku = item.select_one('.comic-diamond-sku')
        if sku:
            self.sku: str = sku.text.strip()
        else:
            self.sku = None

        consensus_head = item.find(name='span', text=re.compile('CONSENSUS:'))
        if consensus_head:
            self.consensus = float(consensus_head.find_next_sibling().strong.text)
        else:
            self.consensus = None

        potw_head = item.find(name='span', text=re.compile('POTW'))
        self.pick_of_the_week = float(potw_head.find_next_sibling().text.rstrip('%'))

        title_anchor = item.select_one('.comic-title > a')
        self.title: str = title_anchor.text
        self.link = title_anchor['href']

        details = item.select_one('.comic-details')
        self.publisher: str = details.strong.text

        parts = self.RELEASE_PAT.match(list(details.strings)[2]).groupdict()
        self.pub_date: date = (
            datetime.strptime(
                f'{parts["year"]}-{parts["month"]}-{parts["day"]}',
                '%Y-%b-%d'
            )
            .date()
        )
        price = parts.get('price')
        if price is None:
            self.price = price
        else:
            self.price = float(price)

        self.desc: str = list(item.select_one('.comic-description > p').strings)[0]

class ComicScraper:
    URL = 'https://leagueofcomicgeeks.com/'

    def __init__(self):
        self.session = Session()

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        self.session.close()

    @staticmethod
    def _parse(content: str) -> Iterable[Comic]:
        soup = BeautifulSoup(content, 'html.parser')
        list_items = soup.select('#comic-list > ul > li')
        return (Comic(li) for li in list_items)

    def get_from_page(self) -> Iterable[Comic]:
        with self.session.get(self.URL + 'comics/new-comics') as response:
            response.raise_for_status()
            return self._parse(response.content)

    def get_from_xhr(self, req_date: date) -> Iterable[Comic]:
        params = {
            'addons': 1,
            'list': 'releases',
            'list_option': '',
            'list_refinement': '',
            'date_type': 'week',
            'date': f'{req_date:%d/%m/%Y}',
            'date_end': '',
            'series_id': '',
            'user_id': 0,
            'title': '',
            'view': 'list',
            'format[]': (1, 6),
            'character': '',
            'order': 'pulls',
        }
        with self.session.get(self.URL + 'comic/get_comics', params=params) as response:
            response.raise_for_status()
            return self._parse(response.json()['list'])

def print_comics(comics: Iterable[Comic]):
    print(f'{"Title":40} {"Publisher":20} {"Date":10} {"Price":6}')

    for c in comics:
        print(
            f'{c.title[:40]:40} {c.publisher[:20]:20} '
            f'{c.pub_date}', end=' '
        )
        if c.price is not None:
            print(f' ${c.price:5.2f}', end='')
        print()

def main():
    parser = argparse.ArgumentParser()
    # Titles of comicbooks i.e "Detective Comics #1"
    parser.add_argument('-t', '--titles', help='Print these comic titles ONLY',
                        action='append')
    args = parser.parse_args()
    titles = args.titles and set(args.titles)

    with ComicScraper() as scraper:
        comics = scraper.get_from_xhr(date(year=2020, month=3, day=25))
        if titles:
            comics = (c for c in comics if c.title in titles)
        print_comics(comics)

if __name__ == '__main__':
    main()

Points:

Your original class is not useful as a class - it's effectively a function; the important stuff to capture in a class is distinct fields on the data you're trying to represent
You can still class-ify the generator as a class method
Use type hints
Use set membership check for titles
Use a regex to parse the info field
Pull out price and date as a float and date, respectively; don't leave them stringly-typed

Note the second method to use an XHR backend instead of the web front-end. The return format is awkward - they return rendered HTML as a part of the JSON payload - but the interface is more powerful and the method might be more efficient. I have not done a lot of investigation into what each of those parameters means; to learn more you will probably have to dig around the site using developer tools.